Boost C++ Libraries: Ticket #9787: [windows] Small duration value passed down to basic_timed_mutex::try_lock_until and condition_variable::wait_until can cause infinite or near infinite wait for win32

viboes — Tue, 18 Mar 2014 21:46:44 GMT

Is this a duplicate of #9618?

owner, status changed

viboes — Tue, 18 Mar 2014 21:47:50 GMT

owner changed from Anthony Williams to viboes
status new → assigned

Tue, 18 Mar 2014 22:15:31 GMT

This ticket does not describe an issue with thread::try_join_for or thread::try_join_until so it is not a duplicate of #9618. However what's described in comment 9 of that ticket sounds similar in nature. The patch in comment 10 of that ticket would not fix this problem.

summary changed

viboes — Sat, 19 Apr 2014 11:12:36 GMT

summary Small duration value passed down to basic_timed_mutex::try_lock_until and condition_variable::wait_until can cause infinite or near infinite wait for win32 → [windows] Small duration value passed down to basic_timed_mutex::try_lock_until and condition_variable::wait_until can cause infinite or near infinite wait for win32

viboes — Sat, 19 Apr 2014 11:13:01 GMT

Have you tried it?

Wed, 23 Apr 2014 15:24:06 GMT

No because we are not calling try_join_for and basic_timed_mutex::try_lock_until and condition_variable::wait_until don't call try_join_for.

Fri, 25 Apr 2014 21:06:43 GMT

It appears that this problem is fixed for the condition_variable::wait_until method in version 1.55.

The problem still exists for basic_timed_mutex::try_lock_until in 1.55.

viboes — Sun, 14 Sep 2014 20:21:53 GMT

Could someone try this patch

git diff  include/boost/thread/win32/basic_timed_mutex.hpp
diff --git a/include/boost/thread/win32/basic_timed_mutex.hpp b/include/boost/thread/win32/basic_timed_mutex.hpp
index b55affd..d20c658 100644
--- a/include/boost/thread/win32/basic_timed_mutex.hpp
+++ b/include/boost/thread/win32/basic_timed_mutex.hpp
@@ -203,7 +203,12 @@ namespace boost
                   do
                   {
-                      chrono::milliseconds rel_time= chrono::ceil<chrono::milliseconds>(tp-chrono::system_clock::now());
+                      chrono::time_point<chrono::system_clock, chrono::system_clock::duration> now = chrono::system_clock::now();
+                      if (tp<=now) {
+                        BOOST_INTERLOCKED_DECREMENT(&active_count);
+                        return false;
+                      }
+                      chrono::milliseconds rel_time= chrono::ceil<chrono::milliseconds>(tp-now);
                       if(win32::WaitForSingleObjectEx(sem,static_cast<unsigned long>(rel_time.count()),0)!=0)
                       {

Niall Douglas — Wed, 17 Sep 2014 11:09:17 GMT

I have not been able to replicate this issue with Boost 1.56 release. See the attached test case - it tests both of the issues reported by the OP and soak tests the timed_mutex for thirty seconds across 128 threads. Runs flawlessly without any fixes.

OP - please supply a version of the attached test case test_9787 which demonstrates the problem you report. Else we shall close this issue as invalid.

Niall

attachment set

Niall Douglas — Wed, 17 Sep 2014 11:09:51 GMT

attachment → test_9787.cpp

Niall Douglas — Wed, 17 Sep 2014 11:11:57 GMT

FYI I tested on Win8.1 x86 using VS2013 Update 1.

viboes — Thu, 18 Sep 2014 05:40:29 GMT

Replying to ned14:

FYI I tested on Win8.1 x86 using VS2013 Update 1.

Thanks Niall.

Even if the issue is not reproducible in your configuration, could you try the patch to see if there is no regressions on the Thread tests?

Niall Douglas — Thu, 18 Sep 2014 13:24:29 GMT

With the patch applied, all unit tests pass including test_9787 on Win8.1 x64 VS2013 Update 1.

viboes — Fri, 19 Sep 2014 17:03:52 GMT

Briand please, could you try the patch?

viboes — Fri, 19 Sep 2014 17:04:11 GMT

Replying to ned14:

With the patch applied, all unit tests pass including test_9787 on Win8.1 x64 VS2013 Update 1.

Thanks Niall.

Fri, 19 Sep 2014 17:44:45 GMT

I'm not sure if "Briand" is me, but I will test this the week of September 22nd.

milestone changed

viboes — Tue, 23 Sep 2014 00:15:50 GMT

milestone To Be Determined → Boost 1.57.0

Tue, 23 Sep 2014 11:59:19 GMT

The fix looks good.

I would prefer to call WaitForSingleObjectEx() with a 0ms timeout in the case of tp<=now but that's up to you.

viboes — Tue, 23 Sep 2014 21:15:31 GMT

Thanks for trying it.

I will commit this patch. If you have any patch that can improve the current code, please post it here on another ticket. As I said, I need some help on the windows platform.

status changed; resolution set

viboes — Fri, 26 Sep 2014 06:08:17 GMT

status assigned → closed
resolution → fixed