microsoft / microsoft/STL

<condition_variable>: condition_variable_any::notify_one/notify_all race with destruction

Open
#883 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

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

Description

In these member functions of condition_variable_any:

https://github.com/microsoft/STL/blob/7447ad59d61f50c13861878f340d051c298458df/stl/inc/condition_variable#L42-L62

Note that wait holds a reference (via shared_ptr) to the internal mutex to ensure it remains alive after _Cnd_wait returns so the lock_guard can unlock it. This is necessary despite that the Standard precludes destruction of the condition_variable_any while threads are blocked waiting because threads are no longer considered to be waiting once they have been signaled even if wait hasn't yet returned.

Also note that notify_one and notify_all lock the internal mutex as well, but they do not hold a reference while calling _Cnd_signal/_Cnd_broadcast. I believe this leaves us vulnerable to a race in which thread A calls _Cnd_signal/_Cnd_broadcast and blocks immediately after waking thread B which knows it is the only waiter so it can "safely" destroy the condition_variable_any before _Cnd_signal/_Cnd_broadcast returns and thread A unlocks the internal mutex.

Is there a requirement I'm missing somewhere that forbids the potential race? The fact that there's no wording to specify that a completion of wait_XXX synchronizes with a call to notify_XXX that wakes that thread hints that the calls to notify and the destructor are potentially concurrent and therefore conflict so they cannot occur (and cause the race) in a conforming program.

Is this a bug?

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 with stl/inc/condition_variable at lines 42–62 and trace the lifetime of the internal mutex through wait, notify_one, notify_all, and destruction. Check the relevant standard requirements and existing synchronization behavior; the issue is resolved when the race is determined and, if confirmed, a safe implementation change and regression coverage are identified.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
operating-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.