Not able to set conditional Variable

Question

in the below code I am waiting inside function waitingForWork() on a condition variable, butdoTheCleanUp() is never called.

#include <stdio.h>
#include <stdlib.h>
#include <signal.h>
#include <iostream>           // std::cout
#include <thread>          // std::thread
#include <mutex>              // std::mutex, std::unique_lock
#include <condition_variable> // std::condition_variable


std::mutex mtx;
std::condition_variable cv;
bool stop=false;

void stopTheWait()
{
    sleep(5);
    printf("stopping the wait.. \n\n");
    std::lock_guard<std::mutex> lck(mtx);
    stop = true;
    cv.notify_all();
}

void doTheCleanUp()/* is never called*/ {
    printf("clean up... \n\n");
}

void waitingForWork(){
    printf("wait for ever... \n\n");

    std::unique_lock<std::mutex> lck(mtx);
    cv.wait(lck, []{ return stop;});
    doTheCleanUp();
    printf("clean Up Done, now end wait... \n\n");
}


int main()
{
    printf("in main... \n");
   
   std::unique_lock<std::mutex> lck(mtx);

   std::thread t1(stopTheWait);
   
   waitingForWork();
   
   printf("exiting main...\n");
   sleep(1);
   
   return 0;
}

FYI: There are C++ versions of those C headers named `cstdio`, `cstdlib` and `csignal`. Prefer using them. — Ted Lyngmo, Nov 05 '20 at 18:11

Remy Lebeau · Accepted Answer · 2020-11-05T18:08:51.370

main() is locking the std::mutex and then calling waitingForWork() in the same thread, which tries to lock the std::mutex again. This is undefined behavior:

If lock is called by a thread that already owns the mutex, the behavior is undefined: for example, the program may deadlock. An implementation that can detect the invalid usage is encouraged to throw a std::system_error with error condition resource_deadlock_would_occur instead of deadlocking.

There is no good reason for main() to obtain that initial lock, get rid of it:

int main()
{
    printf("in main... \n");
   
   //std::unique_lock<std::mutex> lck(mtx); // <-- HERE

   std::thread t1(stopTheWait);
   
   waitingForWork();
   
   printf("exiting main...\n");
   sleep(1);
   
   return 0;
}

Note that in general, if you must lock a mutex multiple times in the same thread, you need to use std::recursive_mutex instead:

A calling thread owns a recursive_mutex for a period of time that starts when it successfully calls either lock or try_lock. During this period, the thread may make additional calls to lock or try_lock. The period of ownership ends when the thread makes a matching number of calls to unlock.

Also note that you need to call t1.join() or t1.detach() before t1 goes out of scope and is destroyed, otherwise its destructor will abortively terminate the calling process:

If *this has an associated thread (joinable() == true), std::terminate() is called.

The C++20 [`std::jthread`](https://en.cppreference.com/w/cpp/thread/jthread) that auto-joins may be worth mentioning as an alternative. — Ted Lyngmo, Nov 05 '20 at 18:13
The main thread must get the lock (before the thread) and keep it until it calls wait. Otherwise the thread may reach notify_all first and then the main thread will be stuck. — Martin York, Nov 05 '20 at 18:39
[what if notify() is called before wait()?](https://stackoverflow.com/questions/17562908/). If `main()` needs to lock the mutex while creating the `thread`, then it needs to unlock the mutex before calling `waitingForWork()`. — Remy Lebeau, Nov 05 '20 at 20:31

score 1 · Answer 2 · answered Nov 05 '20 at 18:42

You have a bug here:

void waitingForWork(){
    printf("wait for ever... \n\n");

    std::unique_lock<std::mutex> lck(mtx);         // This lock is bad.
                                                   // you already locked the
                                                   // mutex in main.
                                                   //
                                                   // locking it again is UB

    cv.wait(lck, []{ return stop;});               // Once you enter wait()
                                                   // lock will be released
                                                   // until you are notified.
    doTheCleanUp();
    printf("clean Up Done, now end wait... \n\n");
}

Not able to set conditional Variable

2 Answers2