Using std::conditional_variable to wait on a condition

11,261

Solution 1

Condition variables wake things up spuriously.

You must have a mutex and it must guard a message of some kind for them to work, or you have zero guarantee that any such wakeup occurred.

This was done, presumably, because efficient implementations of a non-spurious version end up being implemeneted in terms of such a spurious version anyhow.

If you fail to guard the message editing with a mutex (ie, no synchronization on it, the state of the message is undefined behavior. This can cause compilers to optimize the read from memory to skip it after the first read.

Even excluding that undefined behavior (imagine you use atomics), there are race conditions where a message is set, a notification occurs, and nobody waiting on the notification sees the message being set if you fail to have the mutex acquired in the time between the variable being set and the condition variable being notified.

Barring extreme cases, you usually want to use the lambda version of wait.

Auditing condition variable code is not possible unless you audit both the notification code and the wait code.

struct gate {
  bool gate_open = false;
  mutable std::condition_variable cv;
  mutable std::mutex m;

  void open_gate() {
    std::unique_lock<std::mutex> lock(m);
    gate_open=true;
    cv.notify_all();
  }
  void wait_at_gate() const {
    std::unique_lock<std::mutex> lock(m);
    cv.wait( lock, [this]{ return gate_open; } );
  }
};

or

  void open_gate() {
    {
      std::unique_lock<std::mutex> lock(m);
      gate_open=true;
    }
    cv.notify_all();
  }

Solution 2

No, your code will not work.

The mutex protects modifications to the shared variable. As such, all of the waiting threads and the signaling thread must lock that specific mutex instance. With what you've written, each thread has its own mutex instance.

The main reason for all of this mutex stuff is due to the concept of spurious wakeup, an unfortunate aspect of OS implementations of condition variables. Threads waiting on them sometimes just start running even though the condition hasn't been satisfied yet.

The mutex-bound check of the actual variable allows the thread to test whether it was spuriously awoken or not.

wait atomically releases the mutex and starts waiting on the condition. When wait exits, the mutex is atomically reacquired as part of the wakeup process. Now, consider a race between a spurious wakeup and the notifying thread. The notifying thread can be in one of 2 states: about to modify the variable, or after modifying it and about to notify everyone to wake up.

If the spurious wakeup happens when the notifying thread is about to modify the varaible, then one of them will get to the mutex first. So the spuriously awoken thread will either see the old value or the new value. If it sees the new, then it has been notified and will go do its business. If it sees the old, then it will wait on the condition again. But if it saw the old, then it blocked the notifying thread from modifying that variable, so it had to wait until the spurious thread went back to sleep.

Why does std::condition_variable::wait(...) locks the mutex again after a "notify" has been sent to un-sleep it?

Because the mutex locks access to the condition variable. And the first thing you have to do after waking up from a wait call is to check the condition variable. As such, that must be done under the protection of the mutex.

The signalling thread must be prevented from modifying the variable while other threads are reading it. That's what the mutex is for.

Seeing the behaviour in "1)", does that mean that when you do std::condition_variable::notify_all it only makes it so that all of the waiting threads are unblocked/woken up... but in order instead of all at once?

The order they wake up in is not specified. However, by the time notify_all returns, all threads are guaranteed to have been unblocked.

If I only care about threads sleeping until a condition is met and not care a single bit for any mutex acquisition, what can I do?

Nothing. condition_variable requires that access to the actual variable you're checking is controlled via a mutex.

Share:
11,261
Admin
Author by

Admin

Updated on June 04, 2022

Comments

  • Admin
    Admin almost 2 years

    For simplicity, let's assume that we have only one conditional variable to match a single condition that is reflected by a boolean.

    1) Why does std::condition_variable::wait(...) locks the mutex again after a "notify" has been sent to un-sleep it?

    2) Seeing the behaviour in "1)", does that mean that when you do std::condition_variable::notify_all it only makes it so that all of the waiting threads are unblocked/woken up... but in order instead of all at once? If so, what can be done to do it all at once?

    3) If I only care about threads sleeping until a condition is met and not care a single bit for any mutex acquisition, what can I do? Is there an alternative or should current std::condition_variable::wait(...) approach(es) be hacked around this?

    If "hackery" is to be used, will this function work for unblocking all waiting threads on a condition and can it be called from any(per thread) threads:

    //declared somehwere and modified before sending "notify"(ies)
    std::atomic<bool> global_shared_condition_atomic_bool;
    
    //the single(for simplicity in our case) condition variable matched with the above boolean result
    std::condition_variable global_shared_condition_variable;
    
    static void MyClass:wait()
    {
        std::mutex mutex;
        std::unique_lock<std::mutex> lock(mutex);
    
        while (!global_shared_condition_atomic_bool) global_shared_condition_variable.wait(lock);
    }
    

    it would have been called from random "waiting" threads like so:

    void random_thread_run()
    {
        while(someLoopControlValue)
        {
            //random code...
            MyClass:wait(); //wait for whatever condition the class+method is for.
            //more random code...
        }
    }
    

    Edit:

    Gate class

    #ifndef Gate_Header
    #define Gate_Header
    
    #include <mutex>
    #include <condition_variable>
    
    class Gate
    {
    public:
        Gate()
        {
            gate_open = false;
        }
    
        void open()
        {
            m.lock();
            gate_open = true;
            m.unlock();
    
            cv.notify_all();
        }
    
        void wait()
        {
            std::unique_lock<std::mutex> lock(m);
    
            while (!gate_open) cv.wait(lock);
        }
    
        void close()
        {
            m.lock();
            gate_open = false;
            m.unlock();
        }
    
    private:
        std::mutex m;
        std::condition_variable cv;
        bool gate_open;
    };
    
    #endif
    
  • Admin
    Admin over 7 years
    #1 So that mutex is purely to protect access to the very std::condition_variable? #2 So I always have to acquire the mutex from signaling threads BEFORE setting the condition boolean(which now doens't need to be atomic) AND sending the notify event to ensure the notify goes through as a whole? #3 Can I still keep the while(!boolVar) version? I didn't understand why I need to use the more complex 2-argument form, sorry.
  • Yakk - Adam Nevraumont
    Yakk - Adam Nevraumont over 7 years
    @JustinBieber That is because you are a singer, not a programmer. The 2-arg form is simpler, and equivalent; no flow control, just a test function. The mutex protects access to the condition variable and the value being tested. There is a narrow race condition involving not holding the mutex at any point between setting the bool and sending the notification whereby threads starting to wait never wake up.
  • Admin
    Admin over 7 years
    Could you please take a look at the class I wrote as a post edit and confirm it is safe to use in production? I'm very sorry to drag this on but I am having a hard time wrapping my head around it. It's tough for us singers.
  • Yakk - Adam Nevraumont
    Yakk - Adam Nevraumont over 7 years
    @justin I see nothing obvious, but I would never ever directly lock a mutex outside of a locking class. I would use a unique lock and a scope. Also closing the lock is questionable; it looks like code smell to me.