2

Let's say I have something like this:

bool signalled = false;
std::condition_variable cv;
void thread1() {
  while (true) {
    std::unique_lock l(mutex);
    cv.wait_until(l, [] { return signalled; });
    return;
  }
}

void thread2...N() {
  signalled = true;
  cv.notify_all();
}

Is that considered thread-safe? The boolean may be set to true in many threads to interrupt thread1.

Edit: If not thread-safe I’m looking for a description of what the race condition is so I can better understand the underlying issue and fill in a knowledge gap.

Vitali
  • 3,411
  • 2
  • 24
  • 25

1 Answers1

3

Writing to a non-atomic variable without synchronization is UB. However, making signalled an atomic<bool> will not solve the problem.

C++ reference on std::condition_variable reads:

Even if the shared variable is atomic, it must be modified under the mutex in order to correctly publish the modification to the waiting thread.

You should do this:

bool signalled = false;

void thread2...N()
{
    std::unique_lock l(mutex);
    signalled = true;
    cv.notify_all();
}

Related questions:

Evg
  • 25,259
  • 5
  • 41
  • 83
  • 1
    https://stackoverflow.com/questions/41867228/why-do-i-need-to-acquire-a-lock-to-modify-a-shared-atomic-variable-before-noti Clarified it much more for me. The issue is if you have a spurious wake that gets interrupted. You end up reading the stale state (no notify) but then you get signaled (which is a no-op since it’s still checking the condition) and this you miss checking after the value is read. – Vitali Nov 13 '19 at 21:48
  • 1
    Thanks for listening, removed my comments as they not related anymore – Slava Nov 13 '19 at 22:00
  • 1
    Just remark, `notify()` does not have to be called under mutex lock, it can be done after unlock. Either way is right, it may change behavior on high load. – Slava Nov 13 '19 at 22:02