Modify

Opened 9 years ago

Closed 7 years ago

#2704 closed Bugs (fixed)

range lock algorithm fails with iterators

Reported by: jwakely.boost@… Owned by: Anthony Williams
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 (3)

boost_locks.patch (1.9 KB) - added by jwakely.boost@… 9 years ago.
patch to use sfinae in is_mutex_type helpers
boost_locks.2.patch (1.8 KB) - added by jwakely.boost@… 9 years ago.
fixed patch to use sfinae in is_mutex_type helpers
boost_locks3.patch (1.8 KB) - added by jwakely.boost@… 9 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)

Download all attachments as: .zip

Change History (8)

Changed 9 years ago by jwakely.boost@…

Attachment: boost_locks.patch added

patch to use sfinae in is_mutex_type helpers

Changed 9 years ago by jwakely.boost@…

Attachment: boost_locks.2.patch added

fixed patch to use sfinae in is_mutex_type helpers

Changed 9 years ago by jwakely.boost@…

Attachment: boost_locks3.patch added

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 9 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 8 years ago by Jonathan Wakely <jwakely.boost@…>

Cc: jwakely.boost@… added

comment:3 Changed 7 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 7 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 7 years ago by Anthony Williams

Resolution: fixed
Status: newclosed

Fixed on trunk

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Anthony Williams.
The resolution will be deleted.

Add Comment


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

 
Note: See TracTickets for help on using tickets.