This page is a snapshot from the LWG issues list, see the Library Active Issues List for more information and the meaning of New status.

### 3343. Ordering of calls to `unlock()` and `notify_all()` in Effects element of `notify_all_at_thread_exit()` should be reversed

Section: 33.7.3 [thread.condition.nonmember] Status: New Submitter: Lewis Baker Opened: 2019-11-21 Last modified: 2020-02-24

Priority: 3

View all issues with New status.

Discussion:

33.7.3 [thread.condition.nonmember] p2 states:

> Effects: Transfers ownership of the lock associated with `lk` into internal storage and schedules `cond` to be notified when the current thread exits, after all objects of thread storage duration associated with the current thread have been destroyed. This notification shall be as if:

> >     lk.unlock();
> >     cond.notify_all();

One common use-cases for the `notify_all_at_thread_exit()` is in conjunction with `thread::detach()` to allow detached threads to signal when they complete and to allow another thread to wait for them to complete using the `condition_variable/mutex` pair.

However, the current wording for `notify_all_at_thread_exit(condition_variable& cond, unique_lock<mutex> lk)` makes it impossible to know when it is safe to destroy the `condition_variable` in the presence of spurious wake-ups and detached threads.

For example: Consider the following code-snippet:

>     #include <condition_variable>
>     #include <mutex>
>     #include <thread>

>     int main() {
>       std::condition_variable cv;
>       std::mutex mut;
>       bool complete = false;

>       std::thread{[&] {
>         // do work here

>         // Signal thread completion
>         std::unique_lock lk{mut};
>         complete = true;
>         std::notify_all_at_thread_exit(cv, std::move(lk));
>       }}.detach();

>       // Wait until thread completes
>       std::unique_lock lk{mut};
>       cv.wait(lk, [&] { return complete; });

>       // condition_variable destroyed on scope exit
>       return 0;
>     }

This seems to an intended usage of `thread::detach()` and `std::notify_all_at_thread_exit()` and yet this code contains a race involving the call to `cv.notify_all()` on the created thread, and the destructor of the `condition_variable`.

To highlight the issue, consider the following case:

Let `T0` be the thread that executes `main()` and `T1` be the thread created by the `std::thread` construction.

> `T0`: creates thread `T1`
> `T0`: context-switched out by OS
> `T1`: starts running

> `T1`: acquires mutex lock
> `T1`: sets `complete = true`

> `T1`: calls `notify_all_at_thread_exit()`
> `T1`: returns from thread-main function and runs all thread-local destructors
> `T1`: calls `lk.unlock()`
> `T1`: context-switched out by OS
> `T0`: resumes execution
> `T0`: acquires mutex lock
> `T0`: calls `cv.wait()` which returns immediately as `complete` is `true`

> `T0`: returns from `main()`, destroying `condition_variable`
> `T1`: resumes execution

> `T1`: calls `cv.notify_all()` on a dangling `cv` reference (undefined behaviour)

Other sequencings are possible involving spurious wake-ups of the `cv.wait()` call.

A proof-of-concept showing this issue can be found here.

The current wording requires releasing the mutex lock before calling `cv.notify_all()`. In the presence of spurious wake-ups of a `condition_variable::wait()`, there is no way to know whether or not a detached thread that called `std::notify_all_at_thread_exit()` has finished calling `cv.notify_all()`. This means there is no portable way to know when it will be safe for the waiting thread to destroy that `condition_variable`.

However, if we were to reverse the order of the calls to `lk.unlock()` and `cond.notify_all()` then the thread waiting for the detached thread to exit would not be able to observe the completion of the thread (in the above case, this would be observing the assignment of `true` to the `complete` variable) until the mutex lock was released by that thread and subsequently acquired by the waiting thread which would only happen after the completion of the call to `cv.notify_all()`. This would allow the above code example to eliminate the race between a subsequent destruction of the condition-variable and the call to `cv.notify_all()`.

[2019-12-08 Issue Prioritization]

Priority to 3 after reflector discussion.

[2019-12-15; Daniel synchronizes wording with N4842]

[2020-02, Prague]

Response from SG1: "We discussed it in Prague. We agree it’s an error and SG1 agreed with the PR."

Proposed resolution:

This wording is relative to N4842.

1. Change 33.7.3 [thread.condition.nonmember] as indicated:

   >     void notify_all_at_thread_exit(condition_variable& cond, unique_lock<mutex> lk);

   > > […]

   > > -2- Effects: Transfers ownership of the lock associated with `lk` into internal storage and schedules `cond` to be notified when the current thread exits, after all objects of thread storage duration associated with the current thread have been destroyed. This notification is equivalent to:

   > > >     lk.unlock();
   > > >     cond.notify_all();
   > > >     lk.unlock();