7
\$\begingroup\$

The goal is to block all threads that call WaitOne(). When Set() is called, those threads continue. Any calls to WaitOne(), after Set(), return immediately. The challenge is to make sure that all calls to WaitOne() are either "before" or "after" the call to Set(), so that no threads see _state false, but miss the notify_all. I'm looking for feedback on performance and correctness.

#ifndef NO_RESET_EVENT_H
#define NO_RESET_EVENT_H

#include <condition_variable>
#include <atomic>

class NoResetEvent
{
public:
    NoResetEvent() : state(false) {}
    NoResetEvent(const NoResetEvent& other) = delete;

    void WaitOne() {
        std::unique_lock<std::mutex> lock(sync);
        while (!state) {
            underlying.wait(lock);
        }
    }

    void Set() {
        std::unique_lock<std::mutex> lock(sync);
        state = true;
        underlying.notify_all();
    }

private:
    std::condition_variable underlying;
    std::mutex sync;
    volatile bool state;
};

#endif
\$\endgroup\$
6
\$\begingroup\$

Looks good.

The only thing I would change is:

volatile bool _state;

into

 std::atomic<bool>  _state;

The issue with volatile is that it does not universally provide that guarantee that updates are visible to all threads (which std::atomic does).

Could simplify the wait() call.

    while (!_state) {
        _underlying.wait(lock);
    }

    // could be:
    _underlying.wait(lock,[&state](){return state;});

Personally I don't like prefix '_' on identifiers. The rules for using '_' as a prefix are complicated. Even if you know the rules not everybody that reads your code will.

\$\endgroup\$
  • \$\begingroup\$ I sometimes forget to switch naming conventions when switching languages. \$\endgroup\$ – Brent Jan 4 '15 at 2:50
  • \$\begingroup\$ Also, regarding the lambda. I'm thinking that the object construction that goes on in the background might be a slight performance hit. Any insights on this? \$\endgroup\$ – Brent Jan 4 '15 at 2:52
  • 1
    \$\begingroup\$ You are going into a wait() so who cares about performance you are about to stall the processor anyway 99% of the time. But in reality I doubt there is any object created here (but compile with -O3 and check the assembly if you are worried). \$\endgroup\$ – Martin York Jan 4 '15 at 3:26
  • \$\begingroup\$ Regarding synchronization around mutexes: stackoverflow.com/a/11173020/395029 \$\endgroup\$ – Brent Jun 9 '17 at 23:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.