Boost C++ Libraries: Ticket #3585: Boost::thread + valgrind/drd possible false positive

attachment set

Mon, 02 Nov 2009 17:43:58 GMT

attachment → test.cpp

version changed

Mon, 02 Nov 2009 17:45:03 GMT

version Boost 1.40.0 → Boost 1.39.0

anonymous — Tue, 20 Apr 2010 21:23:30 GMT

This is still a problem in 1.42. Has it been determined whether this a bug, or a false positive?

version changed

anonymous — Tue, 20 Apr 2010 21:25:49 GMT

version Boost 1.39.0 → Boost 1.42.0

cc set

bvanassche@… — Sat, 05 Jun 2010 12:05:41 GMT

cc bvanassche@… added

As far as I can see thread_data_base::current_cond is signaled by thread::interrupt() but is never waited upon. That kind of access pattern is safe. This also means that it is safe to remove the member variable thread_data_base::current_cond from the Boost thread library.

viboes — Sun, 04 Dec 2011 17:00:21 GMT

Replying to bvanassche@…:

As far as I can see thread_data_base::current_cond is signaled by thread::interrupt() but is never waited upon. That kind of access pattern is safe. This also means that it is safe to remove the member variable thread_data_base::current_cond from the Boost thread library.

Not yet. Have you take a look at libs/thread/src?

grep -rHn current_cond *
src/pthread/thread.cpp:416:            if(local_thread_info->current_cond)
src/pthread/thread.cpp:419:                BOOST_VERIFY(!pthread_cond_broadcast(local_thread_info->current_cond));

416             if(local_thread_info->current_cond)
417             {
418                 boost::pthread::pthread_mutex_scoped_lock internal_lock(local_thread_info->cond_mutex);
419                 BOOST_VERIFY(!pthread_cond_broadcast(local_thread_info->current_cond));
420             }

bvanassche@… — Mon, 05 Dec 2011 18:45:48 GMT

Do you realize that the above grep output supports my claim that it is safe to remove thread_data_base::current_cond ?

viboes — Sun, 11 Dec 2011 02:58:02 GMT

In boost/thread

grep -rHn current_cond * | grep -v svn
pthread/thread_data.hpp:60:            pthread_cond_t* current_cond;
pthread/thread_data.hpp:67:                current_cond(0)
pthread/thread_data.hpp:104:                    thread_info->current_cond=cond;
pthread/thread_data.hpp:119:                    thread_info->current_cond=NULL;

Note that in line 104 current_cond stores an external condition that comes from the interruption_checker. It is the user of this external condition that will do the wait, isn't it?

grep -rHn interruption_checker * | grep -v svn
pthread/condition_variable.hpp:53:            detail::interruption_checker check_for_interruption(&internal_mutex,&cond);
pthread/condition_variable.hpp:71:            detail::interruption_checker check_for_interruption(&internal_mutex,&cond);
pthread/condition_variable.hpp:135:                detail::interruption_checker check_for_interruption(&internal_mutex,&cond);
pthread/condition_variable.hpp:159:                detail::interruption_checker check_for_interruption(&internal_mutex,&cond);

So IMO the current_cond can not be removed.

cc, owner, status changed

viboes — Sun, 11 Dec 2011 02:58:42 GMT

cc viboes added
owner changed from Anthony Williams to viboes
status new → assigned

type changed

viboes — Sun, 11 Dec 2011 08:54:29 GMT

type Bugs → Support Requests

Why the signaling thread needs to lock the associated mutex?

Moved to support request until resolution clarified.

status, type changed; resolution set

viboes — Thu, 29 Dec 2011 12:35:31 GMT

status assigned → closed
type Support Requests → Bugs
resolution → invalid

Replying to viboes:

Why the signaling thread needs to lock the associated mutex?

Moved to support request until resolution clarified.

BTW, local_thread_info->current_cond is protected already by local_thread_info->cond_mutex

                boost::pthread::pthread_mutex_scoped_lock internal_lock(local_thread_info->cond_mutex);
                BOOST_VERIFY(!pthread_cond_broadcast(local_thread_info->current_cond));

So, for me this is a false positive.