Boost C++ Libraries: Ticket #4882: Win32 shared_mutex does not handle timeouts correctly https://svn.boost.org/trac10/ticket/4882 <p> I have identified two problems with the Win32 implementation of boost::shared_mutex in case of timeouts: </p> <p> Imagine the following scenario: </p> <ol><li>Three threads A, B, C compete for a shared_mutex in exclusive mode with timeouts (timed_lock). </li><li>Thread A acquires the mutex. Thread B and C wait for it in <a class="missing wiki">WaitForMultipleObjects</a>, so exclusive_waiting is 2. </li><li>Thread A decides to release the mutex at the same time that thread B returns from <a class="missing wiki">WaitForMultipleObjects</a> with a timeout. <ol class="loweralpha"><li>Thread A still sees exclusive_waiting == 2 because thread B has not yet adjusted the mutex state. It releases the mutex (exclusive = false), decrements exclusive_waiting to 1 and releases the unlock_sem and exclusive_sem. </li><li>Thread B sees that no threads own the mutex for shared access (shared_count == 0) and exclusive is false. So it sets exclusive to true and now owns the mutex. Notice that it does not do anything with exclusive_waiting, so it is still 1. </li><li>Thread C is woken up due to thread A releasing the semaphores. It loops back to retry to get the mutex but this fails because the mutex is already owned by thread B. However, it still increments exclusive_waiting to 2. </li></ol></li></ol><p> After this process, exclusive_waiting is 2 although only one thread (C) is waiting for the mutex. If this is repeated often enough, timed_lock() will throw a lock_error because exclusive_waiting reached 127 and wrapped to 0. The attached program triggers this problem by simply starting 20 threads which compete for the shared_mutex with a timeout in a loop. On my dual-core 3GHz PC, the exception is thrown in less than a minute. </p> <p> There is another similar failure mode: </p> <ol><li>Two threads A and B compete for a shared_mutex in exclusive mode with timeouts (timed_lock). </li><li>Thread A owns the mutex and thread B waits for it in <a class="missing wiki">WaitForMultipleObjects</a> (exclusive_waiting == 1). </li><li>Again thread A releases the mutex at the same time that thread B returns from <a class="missing wiki">WaitForMultipleObjects</a> with a timeout. <ol class="loweralpha"><li>Thread A again still sees exclusive_waiting == 1, so it releases the mutex, decrements exclusive_waiting to zero, and releases the semaphores. </li><li>Thread B again sees that the mutex is available and acquires it without waiting for the semaphores because it already returned from <a class="missing wiki">WaitForMultipleObjects</a> with a timeout. </li></ol></li><li>Thread A tries to acquire the mutex again in exclusive mode. It sees that the mutex is locked, increments exclusive_waiting to 1 and calls <a class="missing wiki">WaitForMultipleObjects</a> to wait for thread B to release the mutex. </li><li>However, the semaphores are still released from the time thread A released the mutex before but thread B never decremented them because it has already returned from <a class="missing wiki">WaitForMultipleObjects</a> with a timeout. </li><li>This means that thread A returns immediately from <a class="missing wiki">WaitForMultipleObjects</a> without a timeout. It tries again to acquire the mutex which is still locked by thread B, and it increments again exclusive_waiting which is now 2. </li></ol><p> This is again a bug because exclusive_waiting is 2 although only thread A waits for the mutex. </p> <p> To be honest, I do not know how to fix this bug. I think the basic problem is that changing the mutex state and releasing the semaphores is not a single atomic operation. A thread which returned from <a class="missing wiki">WaitForMultipleObjects</a> with a timeout cannot know whether another thread decremented exclusive_waiting for it, so it does not know whether or not it has to decrement exclusive_waiting itself to restore the mutex state. </p> en-us Boost C++ Libraries /htdocs/site/boost.png https://svn.boost.org/trac10/ticket/4882 Trac 1.4.3 martin.jerabek@… Mon, 22 Nov 2010 17:44:07 GMT attachment set https://svn.boost.org/trac10/ticket/4882 https://svn.boost.org/trac10/ticket/4882 <ul> <li><strong>attachment</strong> → <span class="trac-field-new">shrmut.cpp</span> </li> </ul> <p> Sample program to reproduce the problem of increasing exclusive_waiting </p> Ticket anonymous Sun, 27 Mar 2011 02:12:08 GMT <link>https://svn.boost.org/trac10/ticket/4882#comment:1 </link> <guid isPermaLink="false">https://svn.boost.org/trac10/ticket/4882#comment:1</guid> <description> <p> Any update on this issue? This is an extremely annoying bug. </p> </description> <category>Ticket</category> </item> <item> <dc:creator>viboes</dc:creator> <pubDate>Sat, 07 Jan 2012 23:38:32 GMT</pubDate> <title>owner, status changed https://svn.boost.org/trac10/ticket/4882#comment:2 https://svn.boost.org/trac10/ticket/4882#comment:2 <ul> <li><strong>owner</strong> changed from <span class="trac-author">Anthony Williams</span> to <span class="trac-author">viboes</span> </li> <li><strong>status</strong> <span class="trac-field-old">new</span> → <span class="trac-field-new">assigned</span> </li> </ul> Ticket viboes Sat, 21 Jan 2012 23:19:30 GMT status, milestone changed; resolution set https://svn.boost.org/trac10/ticket/4882#comment:3 https://svn.boost.org/trac10/ticket/4882#comment:3 <ul> <li><strong>status</strong> <span class="trac-field-old">assigned</span> → <span class="trac-field-new">closed</span> </li> <li><strong>resolution</strong> → <span class="trac-field-new">fixed</span> </li> <li><strong>milestone</strong> <span class="trac-field-old">To Be Determined</span> → <span class="trac-field-new">Boost 1.49.0</span> </li> </ul> <p> Closed as the test is passing now on trunk regression. </p> Ticket viboes Sat, 21 Jan 2012 23:30:30 GMT status changed; resolution deleted https://svn.boost.org/trac10/ticket/4882#comment:4 https://svn.boost.org/trac10/ticket/4882#comment:4 <ul> <li><strong>status</strong> <span class="trac-field-old">closed</span> → <span class="trac-field-new">reopened</span> </li> <li><strong>resolution</strong> <span class="trac-field-deleted">fixed</span> </li> </ul> <p> Let close it when merged to release </p> Ticket viboes Mon, 28 May 2012 16:47:31 GMT milestone changed https://svn.boost.org/trac10/ticket/4882#comment:5 https://svn.boost.org/trac10/ticket/4882#comment:5 <ul> <li><strong>milestone</strong> <span class="trac-field-old">Boost 1.49.0</span> → <span class="trac-field-new">To Be Determined</span> </li> </ul> Ticket viboes Thu, 05 Jul 2012 06:00:50 GMT <link>https://svn.boost.org/trac10/ticket/4882#comment:6 </link> <guid isPermaLink="false">https://svn.boost.org/trac10/ticket/4882#comment:6</guid> <description> <p> Hi, since 1.50 there is the possibility to use a common implementation for shared_mutex defining BOOST_THREAD_PROVIDES_GENERIC_SHARED_MUTEX_ON_WIN or BOOST_THREAD_VERSION=3. </p> <p> Please, could you tell me if this solves the issue? </p> </description> <category>Ticket</category> </item> <item> <dc:creator>viboes</dc:creator> <pubDate>Sun, 08 Jul 2012 16:10:24 GMT</pubDate> <title>status changed; resolution set; milestone deleted https://svn.boost.org/trac10/ticket/4882#comment:7 https://svn.boost.org/trac10/ticket/4882#comment:7 <ul> <li><strong>status</strong> <span class="trac-field-old">reopened</span> → <span class="trac-field-new">closed</span> </li> <li><strong>resolution</strong> → <span class="trac-field-new">wontfix</span> </li> <li><strong>milestone</strong> <span class="trac-field-deleted">To Be Determined</span> </li> </ul> <p> Please reopen if it is present while defining BOOST_THREAD_PROVIDES_GENERIC_SHARED_MUTEX_ON_WIN or BOOST_THREAD_VERSION=3. </p> Ticket viboes Fri, 02 Nov 2012 18:41:10 GMT status changed; milestone set; resolution deleted https://svn.boost.org/trac10/ticket/4882#comment:8 https://svn.boost.org/trac10/ticket/4882#comment:8 <ul> <li><strong>status</strong> <span class="trac-field-old">closed</span> → <span class="trac-field-new">reopened</span> </li> <li><strong>resolution</strong> <span class="trac-field-deleted">wontfix</span> </li> <li><strong>milestone</strong> → <span class="trac-field-new">To Be Determined</span> </li> </ul> <p> The problem is still there on <a class="missing wiki">MacOs</a>/clang/trunk as show the regression test </p> <pre class="wiki"> Test output: Sandia-darwin-clang-trunk-c++11 - thread - test_4882_lib / clang-darwin-trunk Rev 81145 / Fri, 2 Nov 2012 10:21:52 +0000 Report Time: Fri, 2 Nov 2012 17:47:25 +0000 Compile [2012-11-02 10:57:51 UTC]: succeed "/scratch/kbelco/llvm_darwin/build/Release+Asserts/bin/clang++" -x c++ -isystem /scratch/kbelco/llvm_darwin/libcxx/include -U__STRICT_ANSI__ -O0 -g -Wextra -Wno-long-long -pedantic -std=c++11 -stdlib=libc++ -O0 -fno-inline -Wall -pedantic -g -DBOOST_ALL_NO_LIB=1 -DBOOST_CHRONO_STATIC_LINK=1 -DBOOST_SYSTEM_NO_DEPRECATED -DBOOST_SYSTEM_STATIC_LINK=1 -DBOOST_THREAD_BUILD_LIB=1 -DBOOST_THREAD_POSIX -DBOOST_THREAD_THROW_IF_PRECONDITION_NOT_SATISFIED -DBOOST_THREAD_USE_LIB=1 -I".." -c -o "/Volumes/scratch/kbelco/boost/results/boost/bin.v2/libs/thread/test/test_4882_lib.test/clang-darwin-trunk/debug/threading-multi/test_4882.o" "../libs/thread/test/test_4882.cpp" Link [2012-11-02 10:57:52 UTC]: succeed "/scratch/kbelco/llvm_darwin/build/Release+Asserts/bin/clang++" -stdlib=libc++ -stdlib=libc++ -o "/Volumes/scratch/kbelco/boost/results/boost/bin.v2/libs/thread/test/test_4882_lib.test/clang-darwin-trunk/debug/threading-multi/test_4882_lib" "/Volumes/scratch/kbelco/boost/results/boost/bin.v2/libs/thread/test/test_4882_lib.test/clang-darwin-trunk/debug/threading-multi/test_4882.o" "/Volumes/scratch/kbelco/boost/results/boost/bin.v2/libs/thread/test/test_4882_lib.test/clang-darwin-trunk/debug/threading-multi/tss_null.o" "/Volumes/scratch/kbelco/boost/results/boost/bin.v2/libs/chrono/build/clang-darwin-trunk/debug/link-static/threading-multi/libboost_chrono.a" "/Volumes/scratch/kbelco/boost/results/boost/bin.v2/libs/thread/build/clang-darwin-trunk/debug/link-static/threading-multi/libboost_thread.a" "/Volumes/scratch/kbelco/boost/results/boost/bin.v2/libs/system/build/clang-darwin-trunk/debug/link-static/threading-multi/libboost_system.a" -g -L/scratch/kbelco/llvm_darwin/libcxx/lib Run [2012-11-02 10:57:52 UTC]: fail ../libs/thread/test/test_4882.cpp:48 ... ../libs/thread/test/test_4882.cpp:30../libs/thread/test/test_4882.cpp:27 EXIT STATUS: 139 </pre> Ticket viboes Fri, 02 Nov 2012 18:41:31 GMT cc set https://svn.boost.org/trac10/ticket/4882#comment:9 https://svn.boost.org/trac10/ticket/4882#comment:9 <ul> <li><strong>cc</strong> <span class="trac-author">viboes</span> added </li> </ul> Ticket viboes Sun, 04 Nov 2012 00:28:52 GMT cc deleted https://svn.boost.org/trac10/ticket/4882#comment:10 https://svn.boost.org/trac10/ticket/4882#comment:10 <ul> <li><strong>cc</strong> <span class="trac-author">viboes</span> removed </li> </ul> Ticket viboes Sat, 01 Dec 2012 11:40:28 GMT status changed; resolution set; milestone deleted https://svn.boost.org/trac10/ticket/4882#comment:11 https://svn.boost.org/trac10/ticket/4882#comment:11 <ul> <li><strong>status</strong> <span class="trac-field-old">reopened</span> → <span class="trac-field-new">closed</span> </li> <li><strong>resolution</strong> → <span class="trac-field-new">wontfix</span> </li> <li><strong>milestone</strong> <span class="trac-field-deleted">To Be Determined</span> </li> </ul> <p> <a class="changeset" href="https://svn.boost.org/trac10/changeset/81647" title="Thread: force BOOST_THREAD_PROVIDES_GENERIC_SHARED_MUTEX_ON_WIN">[81647]</a> </p> Ticket viboes Sat, 19 Jan 2013 10:57:48 GMT status changed; resolution deleted https://svn.boost.org/trac10/ticket/4882#comment:12 https://svn.boost.org/trac10/ticket/4882#comment:12 <ul> <li><strong>status</strong> <span class="trac-field-old">closed</span> → <span class="trac-field-new">reopened</span> </li> <li><strong>resolution</strong> <span class="trac-field-deleted">wontfix</span> </li> </ul> <p> Reopened as BOOST_THREAD_PROVIDES_GENERIC_SHARED_MUTEX_ON_WIN will not be defined by default and as <a class="closed ticket" href="https://svn.boost.org/trac10/ticket/7906" title="#7906: Bugs: Very bad performance of generic implementation of shared_mutex on windows (closed: wontfix)">#7906</a> states there is a lost in performances when this define is defined. </p> Ticket viboes Sun, 03 Mar 2013 21:45:21 GMT milestone set https://svn.boost.org/trac10/ticket/4882#comment:13 https://svn.boost.org/trac10/ticket/4882#comment:13 <ul> <li><strong>milestone</strong> → <span class="trac-field-new">Boost 1.54.0</span> </li> </ul> Ticket viboes Sat, 23 Mar 2013 02:14:58 GMT status changed; resolution set https://svn.boost.org/trac10/ticket/4882#comment:14 https://svn.boost.org/trac10/ticket/4882#comment:14 <ul> <li><strong>status</strong> <span class="trac-field-old">reopened</span> → <span class="trac-field-new">closed</span> </li> <li><strong>resolution</strong> → <span class="trac-field-new">fixed</span> </li> </ul> <p> Committed revision <a class="changeset" href="https://svn.boost.org/trac10/changeset/83525" title="Thread: merge from trunk. 1.54">[83525]</a>. </p> Ticket