Re: [Boost-bugs] [Boost C++ Libraries] #7461: detail::win32::ReleaseSemaphore may be called with count_to_release equal to 0

Subject: Re: [Boost-bugs] [Boost C++ Libraries] #7461: detail::win32::ReleaseSemaphore may be called with count_to_release equal to 0
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2013-09-15 16:12:30

#7461: detail::win32::ReleaseSemaphore may be called with count_to_release equal
to 0
---------------------------------------------+----------------------
  Reporter:  Jesper Storm Bache <jsbache@…>  |      Owner:  viboes
      Type:  Bugs                            |     Status:  assigned
 Milestone:  To Be Determined                |  Component:  thread
   Version:  Boost 1.51.0                    |   Severity:  Problem
Resolution:                                  |   Keywords:
---------------------------------------------+----------------------

Comment (by viboes):

 I suspect that the following sequence is possible.


 1. thread A call wait and it stands in [1] below. The value for waiters is
 1.
 2. thread B notify_all and after wake the waiters in [2] the scheduler
 choose the ready thread A.
 3. thread A ends the call to do_wait and the entry_manager destructor
 calls. The value for waiters is 0. in [3] the scheduler choose the ready
 thread B.
 4. thread B calls release_waiters() in [4] which calls to release(0).

 Current Code

 {{{
             template<typename lock_type>
             bool do_wait(lock_type& lock,timeout abs_time)
             {
                 relocker<lock_type> locker(lock);

                 entry_manager entry(get_wait_entry());

                 locker.unlock();

                 bool woken=false;
                 while(!woken)
                 {
                     if(!entry->wait(abs_time)) // [1]
                     {
                         return false;
                     }

                     woken=entry->woken();
                 }
                 return woken;
             } // [3]

             void notify_all() BOOST_NOEXCEPT
             {
                 if(detail::interlocked_read_acquire(&total_count))
                 {
                     boost::lock_guard<boost::mutex>
 internal_lock(internal_mutex);
                     if(!total_count)
                     {
                         return;
                     }
                     wake_waiters(total_count); // [2]
                     for(generation_list::iterator it=generations.begin(),
                             end=generations.end();
                         it!=end;++it)
                     {
                         (*it)->release_waiters(); [4]
                     }
                     generations.clear();
                     wake_sem=detail::win32::handle(0);
                 }
             }
 }}}

 I suspect that the call to entry->remove_waiter() inside entry_manager()
 must be protected

 Could some one check if the following patch for
 boost/thread/win32/condition_variable.hpp works


 {{{
 ===================================================================
 --- condition_variable.hpp      (revision 85663)
 +++ condition_variable.hpp      (working copy)
 @@ -191,18 +191,17 @@
              struct entry_manager
              {
                  entry_ptr const entry;
 +                boost::mutex& internal_mutex;

                  BOOST_THREAD_NO_COPYABLE(entry_manager)
 -                entry_manager(entry_ptr const& entry_):
 -                    entry(entry_)
 +                entry_manager(entry_ptr const& entry_, boost::mutex&
 mutex_):
 +                    entry(entry_), internal_mutex(mutex_)
                  {}

                  ~entry_manager()
                  {
 -                  //if(! entry->is_notified()) // several regression
 #7657
 -                  {
 +                    boost::lock_guard<boost::mutex>
 internal_lock(internal_mutex);
                      entry->remove_waiter();
 -                  }
                  }

                  list_entry* operator->()
 @@ -218,7 +217,7 @@
              {
                  relocker<lock_type> locker(lock);

 -                entry_manager entry(get_wait_entry());
 +                entry_manager entry(get_wait_entry(), internal_mutex);

                  locker.unlock();

 }}}

-- 
Ticket URL: <https://svn.boost.org/trac/boost/ticket/7461#comment:16>
Boost C++ Libraries <http://www.boost.org/>
Boost provides free peer-reviewed portable C++ source libraries.