Kari
Kari

Reputation: 1370

unlock the mutex after condition_variable::notify_all() or before?

Looking at several videos and the documentation example, we unlock the mutex before calling the notify_all(). Will it be better to instead call it after?

The common way:

Inside the Notifier thread:

//prepare data for several worker-threads;

//and now, awaken the threads:
std::unique_lock<std::mutex> lock2(sharedMutex);
_threadsCanAwaken = true;

lock2.unlock(); 
_conditionVar.notify_all(); //awaken all the worker threads;

//wait until all threads completed;

//cleanup:
_threadsCanAwaken = false;

//prepare new batches once again, etc, etc

Inside one of the worker threads:

while(true){
    // wait for the next batch:

    std::unique_lock<std::mutex> lock1(sharedMutex);
    _conditionVar.wait(lock1,  [](){return _threadsCanAwaken});
    lock1.unlock(); //let sibling worker-threads work on their part as well

    //perform the final task

    //signal the notifier that one more thread has completed;

    //loop back and wait until the next task
}

Notice how the lock2 is unlocked before we notify the condition variable - should we instead unlock it after the notify_all() ?

Edit

From my comment below: My concern is that, what if the worker spuriously awakes, sees that the mutex is unlocked, super-quickly completes the task and loops back to the start of while. Now the slow-poke Notifier finally calls notify_all(), causing the worker to loop an additional time (excessive and undesired).

Upvotes: 10

Views: 5737

Answers (6)

PaulH
PaulH

Reputation: 134

I am new to C++ threading and had to do some research online for using condition variables. I found the discussion between @DavidSchwartz and @DannyNiu interesting. Since I was not allowed to add a comment to that discussion, I am just offering here another relevant reference from this CppCon video @36:18.
Back to Basics: Concurrency - Arthur O'Dwyer - CppCon 2020

void returnToken(Token t) {
   std::unique_lock lk(mtx_);
   tokens_.push_back(t);
   lk.unlock();
   cv_.notify_one();
}

Here Arthur O'Dwyer says:

Why isn't it a lock guard? It actually could be a lock guard. The reason I made it a unique lock is so that I could manually unlock it here before doing the notify. In previous presentations, I would sometimes reverse those lines and do the notify before the unlock. Semantically, it doesn't matter; but for performance, people keep telling me that it is a good idea to do the unlock before the notify.

Another article also shows unlocking before the notify.
C++ Core Guidelines: Be Aware of the Traps of Condition Variables
The main point of the that article is: Don’t wait without a condition, which is off topic.

Upvotes: 0

DannyNiu
DannyNiu

Reputation: 1487

should we instead unlock it after the notify_all() ?

After reading several related posts, I've formed the opinion that it's purely a performance issue. If OS supports "wait morphing", unlock after; otherwise, unlock before.

I'm adding an answer here to augment that of @DavidSchwartz 's. Particularly, I'd like to clarify his point 1.

If you unlock before you signal, the signal may wake a thread that choose to block on the condition variable after you unlocked. This can lead to a deadlock if you use the same condition variable to signal more than one logical condition. This kind of bug is hard to create, hard to diagnose, and hard to understand. It is trivially avoided by always signaling before unlocking. This ensures that the change of shared state and the signal are an atomic operation and that race conditions and deadlocks are impossible.

  1. The 1st thing I said is that, because it's a CV and not a Mutex, a better term for the so-called "deadlock" might be "sleep paralysis" - a mistake some programs make is that

    • a thread that's supposed to wake

    • went to sleep due to not rechecking the condition it's been waiting for before wait'ng again.

  2. The 2nd thing is that, when waking some other thread(s),

    • the default choice should be broadcast/notify_all (broadcast is the POSIX term, which is equivalent to its C++ counterpart).

    • signal/notify is an optimized special case used for when there's only 1 other thread is waiting.

  3. Finally 3rd, David is adamant that

    • it's better to unlock after notify,

    • because it can avoid the "deadlock" which I've been referring to as "sleep paralysis".

If it's unlock then notify, then there's a window where another thread (let's call this the "wrong" thread) may i.) acquire the mutex, ii.)going into wait, and iii.) wake up. The steps i. ii. and iii. happens too quickly, consumed the signal, leaving the intended (let's call it "correct") thread in sleep.

I discussed this extensively with David, he clarified that only when all 3 points are violated ( 1. condvar associated with several separate conditions and/or didn't check it before waiting again; 2. signal/notify only 1 thread when there're more than 1 other threads using the condvar; 3. unlock before notify creating a window for race condition ), the "sleep paralysis" would occur.

Finally, my recommendation is that, point 1 and 2 are essential for correctness of the program, and fixing issues associated with 1 and 2 should be prioritized over 3, which should only be a augmentative "last resort".

For the purpose of providing reference, manpage for signal/broadcast and wait contains some info from version 3 of Single Unix Specification that gave some explanations on point 1 and 2, and partly 3. Although specified for POSIX/Unix/Linux in C, it's concepts are applicable to C++.

As of this writing (2023-01-31), the 2018 edition of version 4 of Single Unix Specification is released, and the drafting of version 5 is underway.

Upvotes: 1

Yongwei Wu
Yongwei Wu

Reputation: 5582

David's answer seems to me wrong.

First, assuming the simple case of two threads, one waiting for the other on a condition variable, unlocking first by the notifier will not waken the other waiting thread, as the signal has not arrived. Then the notify call will immediately waken the waiting thread. You do not need any special optimizations.

On the other hand, signalling first has the potential of waking up a thread and making it sleep immediately again, as it cannot hold the lock—unless wait morphing is implemented.

Wait morphing does not exist in Linux at least, according to the answer under this StackOverflow question: Which OS / platforms implement wait morphing optimization?

The cppreference example also unlocks first before signalling: https://en.cppreference.com/w/cpp/thread/condition_variable/notify_all

It explicit says:

The notifying thread does not need to hold the lock on the same mutex as the one held by the waiting thread(s). Doing so may be a pessimization, since the notified thread would immediately block again, waiting for the notifying thread to release the lock, though some implementations recognize the pattern and do not attempt to wake up the thread that is notified under lock.

Upvotes: 1

David Schwartz
David Schwartz

Reputation: 182769

There are no advantages to unlocking the mutex before signaling the condition variable unless your implementation is unusual. There are two disadvantages to unlocking before signaling:

  1. If you unlock before you signal, the signal may wake a thread that choose to block on the condition variable after you unlocked. This can lead to a deadlock if you use the same condition variable to signal more than one logical condition. This kind of bug is hard to create, hard to diagnose, and hard to understand. It is trivially avoided by always signaling before unlocking. This ensures that the change of shared state and the signal are an atomic operation and that race conditions and deadlocks are impossible.

  2. There is a performance penalty for unlocking before signaling that is avoided by unlocking after signaling. If you signal before you unlock, a good implementation will know that your signal cannot possibly render any thread ready-to-run because the mutex is held by the calling thread and any thread affects by the condition variable necessarily cannot make forward progress without the mutex. This permits a significant optimization (often called "wait morphing") that is not possible if you unlock first.

So signal while holding the lock unless you have some unusual reason to do otherwise.

Upvotes: 9

Arpit Singh
Arpit Singh

Reputation: 19

As mentioned here : cppreference.com

The notifying thread does not need to hold the lock on the same mutex as the one held by the waiting thread(s); in fact doing so is a pessimization, since the notified thread would immediately block again, waiting for the notifying thread to release the lock.

That said, documentation for wait

At the moment of blocking the thread, the function automatically calls lck.unlock(), allowing other locked threads to continue.

Once notified (explicitly, by some other thread), the function unblocks and calls lck.lock(), leaving lck in the same state as when the function was called. Then the function returns (notice that this last mutex locking may block again the thread before returning).

so when notified wait will re-attempt to gain the lock and in that process it will get blocked again till original notifying thread releases the lock. So I'll suggest that release the lock before calling notify. As done in example on cppreference.com and most importantly

Don't be Pessimistic.

Upvotes: 1

Slava
Slava

Reputation: 44258

should we instead unlock it after the notify_all() ?

It is correct to do it either way but you may have different behavior in different situations. It is quite difficult to predict how it will affect performance of your program - I've seen both positive and negative effects for different applications. So it is better you profile your program and make decision on your particular situation based on profiling.

Upvotes: 3

Related Questions