Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

notify_one() called before the mutex is unlocked

42 views
Skip to first unread message

Shiyao Ma

unread,
Mar 22, 2016, 12:06:55 AM3/22/16
to
Hi,

I often see code like this:


std::mutex mut;
std::conditional_variable cond;
std::queue<int> qi;

// producer
void my_producer() {
std::lock_guard<std::mutex> lg(mut);
// do something on qi
cond.notify_one();
}


// consumer
void my_consumer() {
std::unqiue_lock<std::mutex> ul(mut);
cond.wait(ul, []{return !qi.empty();});
// do something

ul.unlock()
}


Note that when cond.notify_one() is called, the lock_guard still holds the mutex.
So even cond.wait is wake up, it will soon be put to sleep.

I am confused about the above code. Does the above code make sense?


Regards.

rapot...@gmail.com

unread,
Mar 22, 2016, 6:48:49 AM3/22/16
to
вторник, 22 марта 2016 г., 7:06:55 UTC+3 пользователь Shiyao Ma написал:
Hi, When cond.wait(...) is called mutex has unlocked. When some thread handle notify_one() and step out cond.wait() method mutex became locked again.
sorry for my english!=)

Shiyao Ma

unread,
Mar 22, 2016, 10:48:55 AM3/22/16
to
I might not clearly express my question.

Say, in my original code sample, the producer is:

// producer
void my_producer() {
std::lock_guard<std::mutex> lg(mut);
// do something on qi
cond.notify_one();
}

But I think a better one would be:
// producer
void my_producer() {
{
std::lock_guard<std::mutex> lg(mut);
// do something on qi
}
cond.notify_one();
}

In the latter, when the `notify_one' is called, the `lg' has already unlocked the mutex, so cond.wait in the consumer can successfully get the mutex.

However, the former one suffers the problem that when notify_one is called, the mutex has not been released by the lock guard.


My question is, does the former and the latter make a difference?

Seemingly they should, but I often see code written in the former form.

Regards.

Fred.Zwarts

unread,
Mar 22, 2016, 11:32:00 AM3/22/16
to
"Shiyao Ma" schreef in bericht
news:5bba06b2-80a1-43d1...@googlegroups.com...
The problem with the second method is that it creates a race condition.
notify_one is called after unlocking the mutex. So, it is not known who (if
any) has locked the mutex. It could be that the reason to call notif_one
has disappeared because another thread has done something with qi already.
In the first method, notif_one is called with the mutex locked, so
immediately when it unlocks the mutex, one of the waiting threads will get
the mutex and do sometghing useful with qi, without the risk that a third
thread will intervene.

Hans Bos

unread,
Mar 22, 2016, 11:52:25 AM3/22/16
to

"Fred.Zwarts" <F.Zw...@KVI.nl> wrote in message
news:ncrogv$4qi$1...@gioia.aioe.org...
> "Shiyao Ma" schreef in bericht
> news:5bba06b2-80a1-43d1...@googlegroups.com...
>>
>>I might not clearly express my question.
>>
>>Say, in my original code sample, the producer is:
>>
>>// producer
>>void my_producer() {
>> std::lock_guard<std::mutex> lg(mut);
>> // do something on qi
>> cond.notify_one();
>>}
>>
>>But I think a better one would be:
>>// producer
>>void my_producer() {
>> {
>> std::lock_guard<std::mutex> lg(mut);
>> // do something on qi
>> }
>> cond.notify_one();
>>}
>>
...
>
> The problem with the second method is that it creates a race condition.
> notify_one is called after unlocking the mutex. So, it is not known who
> (if any) has locked the mutex. It could be that the reason to call
> notif_one has disappeared because another thread has done something with
> qi already.
> In the first method, notif_one is called with the mutex locked, so
> immediately when it unlocks the mutex, one of the waiting threads will get
> the mutex and do sometghing useful with qi, without the risk that a third
> thread will intervene.

The first method has the same problem. Waking a thread with notify_xxx() and
aquiring the mutex is not an atomic operation.
It is also possible that another thread is waiting for the mutex before
notify_one is called.

So there is no guarantee that the condition still holds.
In http://en.cppreference.com/w/cpp/thread/condition_variable/notify_one the
advice is to call the notify_one() without the lock.

Regards,
Hans.

SG

unread,
Mar 22, 2016, 11:59:30 AM3/22/16
to
Is that so? I'm not sure. Suppose there is one blocked consumer thread
because the queue is empty. Then, the producer enters the critical
section to push an item onto the queue. Then, a second consumer that
wasn't blocked tries to enter the critical section. The producer
invokes not_empty_condition.notify_one() to wake up a potentially
blocked consumer and then unlocks the mutex. Is it guaranteed that the
previously blocked consumer thread will be the first to lock the mutex
or could the 2nd consumer thread win the race?

I was about to write a response similar to yours and that if the
notification is kept inside the critical section it should be fine to
replace the consumer's while loop

std::unique_lock<std::mutex> lck { mut };
while (qi.empty()) {
not_empty_condition.wait(lck);
}
assert(!qi.empty()); // Obviously!

to a simple if:

std::unique_lock<std::mutex> lck { mut };
if (qi.empty()) {
not_empty_condition.wait(lck);
}
assert(!qi.empty()); // True?!

But I'm not so sure about that anymore. Even if the notification is
done while the mutex was locked, another consumer could still win the
race to lock the mutex and "steal" the item off the queue, couldn't
it?

I'd recommend to stick with a while loop to be on the safe side
(protecting yourself from spurious awakenings). And once you use while
loops or the equivalent

std::unique_lock<std::mutex> lck { mut };
not_empty_condition.wait(lck, [&]{ return !qi.empty(); });
assert(!qi.empty()); // Obviously

you could just as well do the notification outside of the critical
section. Whether that makes a difference, I don't know.

Cheers!
SG

Chris M. Thomasson

unread,
Mar 22, 2016, 4:17:26 PM3/22/16
to
> "Shiyao Ma" wrote in message
> news:84adbe2a-758f-4693...@googlegroups.com...

[...]

Signaling/broadcasting the condvar while the mutex
is not held is fine: It can be more efficient.
However, on systems that have wait morphing,
signaling/broadcasting while the mutex is held
should not be less efficient.


Chris M. Thomasson

unread,
Mar 22, 2016, 4:20:48 PM3/22/16
to
> "Chris M. Thomasson" wrote in message
> news:ncs985$13lk$1...@gioia.aioe.org...
FWIW, I recommend the excellent book:

https://books.google.com/books?id=_xvnuFzo7q0C&pg=PA82&lpg=PA82#v=onepage&q&f=false
(linked to a relevant section)

Chris M. Thomasson

unread,
Mar 22, 2016, 5:46:34 PM3/22/16
to
> "Chris M. Thomasson" wrote in message
> news:ncs9e6$13vl$1...@gioia.aioe.org...

[...]

> FWIW, I recommend the excellent book:

> https://books.google.com/books?id=_xvnuFzo7q0C&pg=PA82&lpg=PA82#v=onepage&q&f=false
> (linked to a relevant section)

Damn! the link above does not seem to work now.
Oh well, here is the book, but the relevant section
does not come up:

https://books.google.com/books?id=TXiJDj9kbiAC

Sorry about that non-sense! ;^o

0 new messages