microsoft / microsoft/STL

<condition_variable>: std::condition_variable::wait_for() returns bogus std::cv_status::no_timeout status

Open
#1,255 11 comments 8 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug vNext
Dominant language
C++
Stars
11.1k
Forks
1.7k
Avg merge
4d 15h
Merged PRs (30d)
22

Description

Describe the bug
Port of DevCom-193041
int do_wait(_Cnd_t cond, _Mtx_t mtx, const xtime *target) in cond.c uses a heuristic _Xtime_diff_to_millis2 to figure out if a timeout occurred. It generates many false std::cv_status::no_timeout. Instead, the function should be using the result of the implementation's call to SleepConditionVariableCS to return the correct status. This flaw causes the user to falsely detect spurious wakeups by looking at the std::cv_status::no_timeout and the predicate. Where 'predicate' is whatever condition the user is trying to wakeup to.

Command-line test case

#include <chrono>
#include <condition_variable>
#include <iostream>
#include <mutex>
#include <windows.h>
#include <thread>

int main() {
    std::mutex              m;
    std::condition_variable c;

    unsigned int bogusNoTimeoutCount = 0;
    for (int i = 0; i < 200; ++i)
    {
        std::unique_lock<std::mutex> l{ m };
        SetLastError(ERROR_SUCCESS);
        // Note that we will only have proper timeouts or super rare spurious timeouts here.
        auto s = c.wait_for(l, std::chrono::milliseconds{ 10 });
        if (s == std::cv_status::no_timeout && GetLastError() == ERROR_TIMEOUT)
        {
            ++bogusNoTimeoutCount;
        }
    }

    std::cout << "bogusNoTimeoutCount: " << bogusNoTimeoutCount << std::endl;

    return 0;
}
C:\temp>cl /MD /EHsc /W4 /WX .\repro.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.28.29115 for x86
Copyright (C) Microsoft Corporation.  All rights reserved.

repro.cpp
Microsoft (R) Incremental Linker Version 14.28.29115.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:repro.exe
repro.obj
C:\temp>.\repro.exe
bogusNoTimeoutCount: 1

Running it again:

C:\temp>.\repro.exe
bogusNoTimeoutCount: 0

Expected Behavior
No bogus timeout returned

STL version
Microsoft Visual Studio Enterprise 2019 Preview
Version 16.8.0 Preview 1.0

Additional context
This is a port of Developer Community Feedback Ticket: The Visual C++ 2017 condition_variable implementation of wait_for returns bogus no_timeout status (DevCom-193041) (Internal VSO-564728)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in cond.c at do_wait(_Cnd_t cond, _Mtx_t mtx, const xtime *target), then inspect the SleepConditionVariableCS call and the _Xtime_diff_to_millis2 heuristic described in the issue. Use the supplied Windows command-line reproduction to verify that wait_for reports the correct cv_status without bogus no_timeout results.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
operating-systems
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.