Need to mutex-protect (atomic) assignment sought by condition variable?

Question

I understand how to use condition variables (crummy name for this construct, IMO, as the cv object neither is a variable nor indicates a condition). So I have a pair of threads, canonically set up with Boost.Thread as:

bool awake = false;
boost::mutex sync;
boost::condition_variable cv;
void thread1()
{
    boost::unique_lock<boost::mutex> lock1(sync);
    while (!awake)
        cv.wait(lock1);
    lock1.unlock();    // this line actually not canonical, but why not?
    // proceed...
}
void thread2()
{
    //...
    boost::unique_lock<boost::mutex> lock2;
    awake = true;
    lock2.unlock();
    cv.notify_all();
}

My question is: does thread2 really need to be protecting the assignment to awake? It seems to me the notify_all() call should be sufficient. If the data being manipulated and checked against were more than a simple "ok to proceed" flag, I see the value in the mutex, but here it seems like overkill.

A secondary question is that asked in the code fragment: Why doesn't the Boost documentation show the lock in thread1 being unlocked before the "process data" step?

EDIT: Maybe my question is really: Is there a cleaner construct than a CV to implement this kind of wait?

Mike Seymour · Accepted Answer · 2012-07-18T18:10:09.673

1

does thread2 really need to be protecting the assignment to awake?

Yes. Modifying an object from one thread and accessing it from another without synchronisation gives undefined behaviour. Even if it's just a bool.

For example, on some multiprocessor systems the write might only affect local memory; without an explicit synchronisation operation, other threads might never see the change.

Why doesn't the Boost documentation show the lock in thread1 being unlocked before the "process data" step?

If you unlocked the mutex before clearing the flag, then you might miss another signal.

Is there a cleaner construct than a CV to implement this kind of wait?

In Boost and the standard C++ library, no; a condition variable is flexible enough to handle arbitrary shared state and not particularly over-complicated for this simple case, so there's no particular need for anything simpler.

More generally, you could use a semaphore or a pipe to send a simple signal between threads.

edited Jul 18 '12 at 18:10

answered Jul 18 '12 at 18:04

Mike Seymour

249,747
28
448
644

It's the memory synch thing that keeps tripping me up. Regarding your last point: semaphore (by that name) isn't part of Boost or C++11 threading; Boost.InterProcess has one. It's unclear how it would be simpler. Similarly, I wouldn't use a pipe for a single process. – Mike C Jul 18 '12 at 19:15
@MikeC: That's right, there is no semaphore in the Boost or C++ threading libraries, just as my answer says. Conceptually, a semaphore or pipe might be considered slightly cleaner since it's a single object and doesn't involve any external shared state. Personally, I prefer higher-level synchronisation via message queues and the like, avoiding explicitly shared state, but sadly that didn't make it into this version of the standard. – Mike Seymour Jul 18 '12 at 20:08

score 1 · Answer 2 · answered Jul 18 '12 at 18:15

1

Formally, you definitely need the lock in both threads: if any thread modifies an object, and more than one thread accesses it, then all accesses must be synchronized.

In practice, you'll probably get away with it without the lock; it's almost certain that notify_all will issue the necessary fence or membar instructions to ensure that the memory is properly synchronized. But why take the risk?

As to the absense of the unlock, that's the whole point of the scoped locking pattern: the unlock is in the destructor of the object, so that the mutex will be unlocked even if an exception passes through.

answered Jul 18 '12 at 18:15

James Kanze

150,581
18
184
329

Well, obviously if the follow-on processing is going to perform additional operations that will involve further synching on the same mutex, the unlock has to be used. I'm actually not too fond of scoped locking, for this reason and for the ugly inline-block coding it encourages; I prefer the Java syntax. – Mike C Jul 18 '12 at 19:19
@MikeC: Scoped locking is essential if you do anything that might throw an exception, unless you use an even uglier try/catch/rethrow block - which is why I dislike the Java syntax. – Mike Seymour Jul 18 '12 at 20:40
@MikeSeymour: We'll have to agree to disagree on the relative ugliness of non-keyword-bound open-close brace constructs vs those with keywords like try and catch. Indentation for the sake of introducing scope is simply hackish. :) As an alternative, I also like Python's `with`. – Mike C Jul 18 '12 at 21:00
@MikeC You'll obviously put this synchronization protocol in its own function, which you call before doing the other things. Good functional decomposition requires it, independently of the issue of freeing the lock. As for the Java syntax... locking, in Java, is part of the language, and the way it is implemented means that one function cannot return a locked object. A very important and useful idiom. – James Kanze Jul 19 '12 at 08:00
@MikeC As for using a try/finally rather than a destructor in genral... The destructor places the responsibility in the hands of the author of the object which needs it; the try/catch places it in the hands of the client. One of the most frequent errors in Java code is clients who've forgotten the try/finally for objects which do require "destruction" (e.g. like streams which need closing). – James Kanze Jul 19 '12 at 08:04
@JamesKanze: Ah yes, I forgot that Java has built-in locks. I was thinking of the mess that results from trying to manage other (non-garbage-collected) resources. – Mike Seymour Jul 19 '12 at 08:23

Need to mutex-protect (atomic) assignment sought by condition variable?

2 Answers2