Opened 9 years ago
Closed 8 years ago
#9787 closed Bugs (fixed)
[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
Reported by: | Owned by: | viboes | |
---|---|---|---|
Milestone: | Boost 1.57.0 | Component: | thread |
Version: | Boost 1.53.0 | Severity: | Problem |
Keywords: | WaitForSingleObject timed_mutex try_lock_until wait_until | Cc: |
Description
Creating a unique_lock using a timed_mutex which is passed a small chrono::milliseconds duration value (10 milliseconds in our case) can cause a very large value to be passed to WaitForSingleObject for the dwMilliseconds parameter. The code path creates an initial chrono::time_point where the duration value is added to the current time. Once it gets to try_lock_until it calculates a new rel_time duration to feed to WaitForSingleObject. If the small duration has elapsed due to thread switching then the subtraction results in a very large positive value to be passed to WaitForSingleObject. Similarly, if we calculate our own boost::chrono::system_clock::time_point and pass it to boost::condition_variable::wait_until and the time_point has passed then the subtraction to compute a relative time value can result in a very large positive value. Another possibility is if the caller inadvertently passes an elapsed time_point to condition_variable::wait_until.
Attachments (1)
Change History (20)
comment:2 by , 9 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 9 years ago
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.
comment:4 by , 9 years ago
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 |
---|
comment:6 by , 8 years ago
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.
comment:7 by , 8 years ago
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.
comment:8 by , 8 years ago
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) {
comment:9 by , 8 years ago
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
by , 8 years ago
Attachment: | test_9787.cpp added |
---|
comment:11 by , 8 years ago
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?
follow-up: 14 comment:12 by , 8 years ago
With the patch applied, all unit tests pass including test_9787 on Win8.1 x64 VS2013 Update 1.
comment:14 by , 8 years ago
Replying to ned14:
With the patch applied, all unit tests pass including test_9787 on Win8.1 x64 VS2013 Update 1.
Thanks Niall.
comment:15 by , 8 years ago
I'm not sure if "Briand" is me, but I will test this the week of September 22nd.
comment:16 by , 8 years ago
Milestone: | To Be Determined → Boost 1.57.0 |
---|
comment:17 by , 8 years ago
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.
comment:18 by , 8 years ago
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.
comment:19 by , 8 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Is this a duplicate of #9618?