CODEWITHSUNDEEP
c++multithreadingcondition-variable
Junekey Jeon
Junekey Jeon

Reputation: 1587

std::condition_variable::notify_one() does not wake up a waiting thread

I have a code something like the following:

#include <mutex>
#include <condition_variable>
#include <future>

std::mutex mtx;
std::condition_variable cv;
std::future<void> worker;

void worker_thread() {
  {
    std::lock_guard<std::mutex> lg{ mtx };
    // do something 1
  }
  cv.notify_one();

  // do something 2
}

int main() {
  {
    std::unique_lock<std::mutex> lg{ mtx };
    worker = std::async(std::launch::async, worker_thread);      
    cv.wait(lg);
  }
  // do something 3
}

The main thread does not proceed to // do something 3 and I can't understand why. I thought the line cv.notify_one() should be reached from the worker thread after cv.wait(lg) has been passed by the main thread, so there is no reason to hang.

The worker thread is responsible for some streaming data processing, while the main thread is mainly responsible for GUI event processing.

// do something 1 is about some initialization that should be done inside the worker thread. The main thread should wait for the worker thread to finish it.

// do something 2 is the main job of the worker thread.

// do something 3 is the main job of the main thread.

Changing cv.notify_one() to cv.notify_all() didn't help.

Is this usage of condition variable correct?

Upvotes: 4

Views: 2993

Answers (2)

Junekey Jeon
Junekey Jeon

Reputation: 1587

I'm sorry. The question was totally wrong. I think the claim "notification should be fired after the main thread have begun to wait" is still right, but the root cause of the hang was spurious awakening, as Jive Dadson and Erik Alapää pointed out.

There was a reason I couldn't compile the code w/o optimization options, so I misinterpreted the point of hang because the point where the debugger was pointing to was not very clear. The point of hang was not the line cv.wait(lg). It was somewhere inside // do something 3.

I have a flag that is set if // do something 1 has been succeeded, while it is cleared if an exception is thrown inside // do something 1. In // do something 3, the flag is checked and if it indicates failure, the main thread calls worker.get() to rethrow the exception. Since // do something 2 is a form of infinite loop, the main thread just hangs if cv has waken up spuriously so that the flag is not yet set.

Now it works fine! Thanks everyone.

Upvotes: 0

Jive Dadson
Jive Dadson

Reputation: 17056

I have to backtrack on my original answer, and I apologize to Junekey for that. I misread the code, and concluded there was a race condition. I cannot reproduce the problem. We need an example that actually blocks forever on the cv.wait in order to figure out why it is doing so. Nevertheless, the code is incorrect if for no other reason than that it could get a spurious notification and pass through cv.wait before the worker_thread calls cv.notify. That rarely happens, but it does happen.

This code is more or less canonical:

#include <mutex>
#include <condition_variable>
#include <thread>

std::mutex mtx;
std::condition_variable cv;
bool worker_done = false;  // <<< puts the "condition" in condition_variable

void worker_thread() {

    // do something 1

    {
        std::lock_guard<std::mutex> lg{ mtx };
        worker_done = true;  // ... and whatever
    }

    cv.notify_one();

    // do something 2
}

int main() {
    std::thread workman(worker_thread);
    {
        std::unique_lock<std::mutex> lg{ mtx };
        while (!worker_done) {
            cv.wait(lg);
        }
    }
    // do something 3
    workman.join();
}

Upvotes: 2

Related Questions