Stl: <condition_variable>: condition_variable_any::wait_for(0) does not unlock the lock

Created on 11 Dec 2019  路  3Comments  路  Source: microsoft/STL

Describe the bug
In condition_variable_any, if the timeout is already expired, we immediately return with a timeout status:

https://github.com/microsoft/STL/blob/aa0a7a3d859ade0f6f1ff13aa4ef74b3d5ce2326/stl/inc/condition_variable#L74-L76

However, the standard depicts condition_variable(_any)? as unlocking the lock unconditionally as their first action: https://eel.is/c++draft/thread.condition.condvarany#thread.condvarany.wait-6

For condition_variable, this is unobservable; the user doesn't get to inspect what std::mutex does, so we can optimize that unlock and immediate lock as a no-op. (Indeed, even if we did not do this, Windows' locks are 'unfair' meaning that's the likely outcome even if we were to try to unlock and relock). However, for condition_variable_any, the user might be using a fair lock, or might even just log attempts to unlock and relock, so we should be calling their unlock even for an already expired timeout.

Also tracked by Developer Community as DevCom-733115, and Microsoft-internal VSO-991199.

bug fixed high priority

Most helpful comment

I would avoid making large structural changes in the name of perf unless you have a benchmark showing the difference (or the changes are 'obvious' cleanup). We do not have a specific perf test harness or the tech to make such a thing, since standard library perf depends entirely on what user code does with it, and we don't know what user code looks like on average.

All 3 comments

@BillyONeal @StephanTLavavej: Looking at <condition_variable>, the locking/unlocking part is missing in wait_until and wait_for. I am thinking the more maintainable code will call wait_until from wait_for, but that could have performance cost. What's the guidance on that? Is there a performance measurement harness? Should I repeat code for the sake of performance? I would think yes, but confirming.

Also, is there a different issue to track adding interruptible waits?

template<class Lock, class Rep, class Period, class Predicate>
      bool wait_for(Lock& lock, stop_token stoken,
                    const chrono::duration<Rep, Period>& rel_time, Predicate pred);

I would avoid making large structural changes in the name of perf unless you have a benchmark showing the difference (or the changes are 'obvious' cleanup). We do not have a specific perf test harness or the tech to make such a thing, since standard library perf depends entirely on what user code does with it, and we don't know what user code looks like on average.

Also, is there a different issue to track adding interruptible waits?

https://github.com/microsoft/STL/issues/32

Was this page helpful?
0 / 5 - 0 ratings