bug in pthreads implementation of recursive_mutex
Hi, It looks that there is a bug in .../boost_1_30_0/libs/thread/src/recursive_mutex.cpp. The bug shows in the pthreads implementation of the recursive_mutex when the mutex is used with the "condition" synchronization primitive. Here is the description of the bug: When the client code calls wait() function on a condition sending as a parameter a lock object associated with a recursive_mutex ("m_Condition.wait(Lock)"), the following functions get called: boost::condition::wait(L& Lock) at condition.hpp:91 boost::condition::do_wait(M& mutex) at condition.hpp:137 boost::detail::thread::lock_opsboost::recursive_mutex::unlock(Mutex& m, lock_state& state) at lock.hpp:54 boost::recursive_mutex::do_unlock(cv_state& state) The implementation of the last function on "winthreads" and "mptasks" resets the 'm_count' data member to zero. The "pthreads" implementation at recursive_mutex.cpp:399 doesn't reset the 'm_count' to zero. As a result, when the calling thread enters pthread_cond_wait() and the mutex is unlocked by the pthreads the 'm_count' still remains non-zero. Starting from this point, the things may go bad. Consider, for example, that another thread tries to acquire the mutex. The thread calls: boost::recursive_mutex::scoped_lock Lock(m_Mutex) boost::detail::thread::scoped_lockboost::recursive_mutex::scoped_lock<boos t::recursive_mutex>(Mutex& mx, initially_locked = true) at lock.hpp:66 boost::detail::thread::scoped_lockboost::recursive_mutex::lock() at lock.hpp:76 boost::detail::thread::lock_opsboost::recursive_mutex::lock(Mutex& m) at lock.hpp:34 boost::recursive_mutex::do_lock() at recursive_mutex.cpp:307 The last function locks the mutex again, and, if BOOST_HAS_PTHREAD_MUTEXATTR_SETTYPE is defined, increments the counter to 2. Now, since the count is more than 1, the function unlocks the mutex. As a result, the mutex remains unlocked, after the lock function is called. The same problem exists in recursive_try_mutex and recursive_timed_mutex. I've tried to fix the code as follows, which seemed to help in my particular case: ---------------------------------------------------------------------------- ------------------------------------ *** recursive_mutex.cpp.orig Sun Feb 23 19:38:11 2003 --- recursive_mutex.cpp Sat May 3 17:32:44 2003 *************** *** 412,417 **** --- 412,418 ---- state.pmutex = &m_mutex; state.count = m_count; + m_count = 0; } recursive_try_mutex::recursive_try_mutex() *************** *** 608,613 **** --- 609,615 ---- state.pmutex = &m_mutex; state.count = m_count; + m_count = 0; } recursive_timed_mutex::recursive_timed_mutex() *************** *** 787,792 **** --- 789,795 ---- state.pmutex = &m_mutex; state.count = m_count; + m_count = 0; } #elif defined(BOOST_HAS_MPTASKS) ---------------------------------------------------------------------------- ------------------------------------ Any comments? Is this indeed a bug? Is the fix correct? Thanks, Yulik. [Non-text portions of this message have been removed]
"Feldman, Yulik" wrote: [...]
Any comments?
Info. Read this: http://groups.google.com/groups?threadm=34293AAA.69FB%40opentext.com (Subject: recursive mutexes and conditions) regards, alexander.
Feldman, Yulik said:
Hi,
It looks that there is a bug in .../boost_1_30_0/libs/thread/src/recursive_mutex.cpp. The bug shows in the pthreads implementation of the recursive_mutex when the mutex is used with the "condition" synchronization primitive. Here is the description of the bug:
You're evaluation of the bug is correct. I'll fix this in CVS shortly. Do you maybe have a test case for this that I could add to the regression tests? BTW, Mr. Terekhov does provide a good resource in his posting on this subject. I disagree with some points in the thread he posted, but it is quite true that you absolutely must gaurantee that invariants hold at the point cond.wait() is called, and that doing so is complicated by the use of recursive locks. Either avoid recursive locks entirely, avoid condition variables with recursive mutexes, or be very sure of the design and the state of your invariants. -- William E. Kempf
participants (3)
-
Alexander Terekhov
-
Feldman, Yulik
-
William E. Kempf