0

Consider the following example class, which allows one thread to wait for a signal from another thread.

class Sync
{
    std::mutex mtx, access;
    std::condition_variable cv;
    bool waiting;

public:
    Sync()
        : waiting(false)
    {
    }
    Sync(const Sync&);
    ~Sync()
    {
        sendSignal();
    }

    void waitForSignal()
    {
        access.lock();
        if (!waiting)
        {
            std::unique_lock<std::mutex> lck (mtx);
            waiting = true;
            access.unlock();

            // in the time between these two statements, 'sendSignal()' acquires 
            // the lock and calls 'cv.notify_all()', thus the signal was missed. 

            cv.wait(lck);
        }
        else
            access.unlock();
    }
    void sendSignal()
    {
        access.lock();
        if (waiting)
        {
            std::unique_lock<std::mutex> lck (mtx);
            cv.notify_all();
            waiting = false;
        }
        access.unlock();
    }
};

The problem I'm having is that a signal will occasionally be missed due to interleaving during the time between unlocking the 'access' mutex and calling 'wait()' on the condition_variable. How can I prevent this?

Madison Brown
  • 331
  • 3
  • 10
  • Looks like you need a _condition variable_. – Branko Dimitrijevic Aug 18 '14 at 18:28
  • I am using a condition variable. – Madison Brown Aug 18 '14 at 18:29
  • This stack overflow on critical regions may help. http://stackoverflow.com/questions/4434050/multithreading-and-critical-sections-use-c – Richard Chambers Aug 18 '14 at 18:29
  • @MadisonBrown This [answer](http://stackoverflow.com/a/4793662/1413395) provides a reasonable implementation of a semaphore surrogate using a condition variable. I think the boost stuff can be easily mapped to the appropriate c++11 features. – πάντα ῥεῖ Aug 18 '14 at 18:34
  • 2
    You don't, generally, want to wait for a "signal" from another thread, or any other momentary event - precisely because such an event can easily be missed. Instead, you want to wait for some condition to become true (e.g. "data available for reading"). You check the condition, and wait only if it's false. A signal from another thread is then just a means to get you to wake up and check the condition again. A condition variable neatly encapsulates this pattern. – Igor Tandetnik Aug 18 '14 at 18:40

1 Answers1

2

You should probably only have one mutex. I don't see why you need access. Use mtx to protect the waiting variable and for the condition variable.

class Sync
{
    std::mutex mtx;
    std::condition_variable cv;
    bool waiting;

public:
    Sync()
        : waiting(false)
    {
    }
    Sync(const Sync&);
    ~Sync()
    {
        sendSignal();
    }

    void waitForSignal()
    {
        std::unique_lock lck (mtx);
        if (!waiting)
        {
            waiting = true;

            cv.wait(lck);
        }
    }
    void sendSignal()
    {
        std::unique_lock lck (mtx);
        if (waiting)
        {
            cv.notify_all();
            waiting = false;
        }
    }
};

The waiting variable and the condition variable state are tied together so they should be treated as a single critical section.

dohashi
  • 1,771
  • 8
  • 12