Modify

Ticket #2704 (closed Bugs: fixed)

Opened 5 years ago

Last modified 4 years ago

range lock algorithm fails with iterators

Reported by: jwakely.boost@… Owned by: anthonyw
Milestone: Boost 1.38.0 Component: thread
Version: Boost 1.37.0 Severity: Problem
Keywords: Cc: jwakely.boost@…

Description

#include <boost/thread.hpp>

#include <vector>

int main() {

std::vector<boost::mutex> v;

boost::lock(&v.front(), &v.front()+v.size()); boost::lock(v.begin(), v.end());

return 0;

}

With GCC 4.3.2 and 4.4 this fails to compile, complaining that std::vector<mutex>::iterator does not have members lock, unlock and try_lock e.g. /home/redi/src/boost/boost/thread/locks.hpp: In static member function ‘static char boost::detail::has_member_try_lock<T>::has_member(U*, bool (U::*)()) [with U = gnu_cxx::normal_iterator<boost::mutex*, std::vector<boost::mutex, std::allocator<boost::mutex> > >, T = gnu_cxx::normal_iterator<boost::mutex*, std::vector<boost::mutex, std::allocator<boost::mutex> > >]’: /home/redi/src/boost/boost/thread/locks.hpp:75: instantiated from ‘const bool boost::detail::has_member_try_lock<gnu_cxx::normal_iterator<boost::mutex*, std::vector<boost::mutex, std::allocator<boost::mutex> > > >::value’ /home/redi/src/boost/boost/thread/locks.hpp:84: instantiated from ‘const bool boost::is_mutex_type<gnu_cxx::normal_iterator<boost::mutex*, std::vector<boost::mutex, std::allocator<boost::mutex> > > >::value’ /home/redi/src/boost/boost/thread/locks.hpp:1133: instantiated from ‘void boost::lock(const MutexType1&, const MutexType2&) [with MutexType1 = gnu_cxx::normal_iterator<boost::mutex*, std::vector<boost::mutex, std::allocator<boost::mutex> > >, MutexType2 = gnu_cxx::normal_iterator<boost::mutex*, std::vector<boost::mutex, std::allocator<boost::mutex> > >]’ lock_range.cc:10: instantiated from here

The commented-out line above compiles fine, using mutex* as the Iterator type rather than std::vector<mutex>::iterator.

I don't think SFINAE applies in has_member_try_lock<T>::has_member<U>, because bool(U::*)() is a valid type for iterators classes, so substitution succeeds, but &U::try_lock is not a valid expression.

In the pointer case where U is a pointer, bool(U::*)() is not a valid type, so substitution fails, but is not an error.

The attached patch (against trunk) adds another default argument to has_member_{lock,try_lock,unlock}::has_member with a type that depends on the presence of a member with the right name. I think this would still fail to compile if used with an iterator type that had a member lock/try_lock/unlock with a different signature to that expected by the Lockable concept.

Jonathan

Attachments

boost_locks.patch Download (1.9 KB) - added by jwakely.boost@… 5 years ago.
patch to use sfinae in is_mutex_type helpers
boost_locks.2.patch Download (1.8 KB) - added by jwakely.boost@… 5 years ago.
fixed patch to use sfinae in is_mutex_type helpers
boost_locks3.patch Download (1.8 KB) - added by jwakely.boost@… 5 years ago.
take 3! this one's simpler, and doesn't fail if the iterator type happens to have a member like int Iterator::lock(int)

Change History

Changed 5 years ago by jwakely.boost@…

patch to use sfinae in is_mutex_type helpers

Changed 5 years ago by jwakely.boost@…

fixed patch to use sfinae in is_mutex_type helpers

Changed 5 years ago by jwakely.boost@…

take 3! this one's simpler, and doesn't fail if the iterator type happens to have a member like int Iterator::lock(int)

comment:1 Changed 5 years ago by jwakely.boost@…

gah, I've just noticed that trac concatenated the lines in the original test case I posted, so that the problem line was commented out. It should be:

boost::lock(&v.front(), &v.front()+v.size());

boost::lock(v.begin(), v.end());

the second line is the problem.

Ignore the first patch, it's full of copy&paste errors. The second one fixes the original test case above, but fails for an iterator type like:

struct iterator : std::vector<boost::mutex>::iterator {

template<typename T> iterator(T t) : std::vector<boost::mutex>::iterator(t) { }

iterator() : std::vector<boost::mutex>::iterator() { }

iterator& operator++();

iterator operator++(int);

bool lock(int); confuses has_member_lock

};

The lock member means substitution succeeds, but the "dummy" parameter fails to compile because the signature of &U::lock doesn't match.

The third patch handles the case above by extending sfinae_type to only succeed for members with the right signature.

comment:2 Changed 5 years ago by Jonathan Wakely <jwakely.boost@…>

  • Cc jwakely.boost@… added

comment:3 Changed 4 years ago by viboes

boost::mutex is not copyable, so you can not put them on std::vector. You will need to wait until there is a common move semantics emulation and the associated containers.

Could we close this ticket, or change it as a Feature request?

comment:4 Changed 4 years ago by Jonathan Wakely <jwakely.boost@…>

The bug is not "mutex is not copyable" it is that the range lock algo doesn't work with iterators. The original testcase does not require mutex to be copyable, because it doesn't insert any mutexes into the vector. But it still fails to compile, because of a bug in boost::lock.

If it helps you understand the problem I'll try to come up with a different test case that uses a type that is Lockable and Copyable, instead of boost::mutex

comment:5 Changed 4 years ago by anthonyw

  • Status changed from new to closed
  • Resolution set to fixed

Fixed on trunk

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.