Search code examples
c++multithreadingcondition-variable

C++ Condition variable to signal end of detached thread execution stalls


I have some code which I'm working on where a detached thread is spawned, does some work, and then should wait for a signal from main() before sending another signal back to main indicating that the thread has quit.

I'm fairly new to condition variables, however I have worked with some multi thread code before. (Mostly mutexes.)

This is what I tried to implement, but it doesn't behave the way I would have expected. (Likely I misunderstood something.)

The idea behind this is to pass a struct containing two flags to each detached thread. The first flag indicates that main() says "it is ok to exit, and drop off the end of the thread function". The second flag is set by the thread itself and signals to main() that the thread has indeed exited. (It's just to confirm the signal from main() is recieved ok and to send something back.)

#include <cstdlib> // std::atoi
#include <iostream>
#include <thread>
#include <vector>
#include <random>
#include <future>
#include <condition_variable>
#include <mutex>

struct ThreadStruct
{

    int id;

    std::condition_variable cv;
    std::mutex m;

    int ok_to_exit;
    int exit_confirm;

};



void Pause()
{
    std::cout << "Press enter to continue" << std::endl;
    std::cin.get();
}


void detachedThread(ThreadStruct* threadData)
{
    std::cout << "START: Detached Thread " << threadData->id << std::endl;
    
    // Performs some arbitrary amount of work.
    for(int i = 0; i < 100000; ++ i);

    std::cout << "FINISH: Detached thread " << threadData->id << std::endl;
    
    std::unique_lock<std::mutex> lock(threadData->m);
    std::cout << "WAIT: Detached thread " << threadData->id << std::endl;
    threadData->cv.wait(lock, [threadData]{return threadData->ok_to_exit == 1;});
    std::cout << "EXIT: Detached thread " << threadData->id << std::endl;
    threadData->exit_confirm = 1;

}

int main(int argc, char** argv)
{
    
    int totalThreadCount = 1;

    ThreadStruct* perThreadData = new ThreadStruct[totalThreadCount];
    std::cout << "Main thread starting " << totalThreadCount << " thread(s)" << std::endl;

    for(int i = totalThreadCount - 1; i >= 0; --i)
    {
        perThreadData[i].id = i;
        perThreadData[i].ok_to_exit = 0;
        perThreadData[i].exit_confirm = 0;
                
        std::thread t(detachedThread, &perThreadData[i]);
        t.detach();
            
    }


    for(int i{0}; i < totalThreadCount; ++i)
    {
        ThreadStruct *threadData = &perThreadData[i];
        
        std::cout << "Waiting for lock - main() thread" << std::endl;
        std::unique_lock<std::mutex> lock(perThreadData[i].m);
        std::cout << "Lock obtained - main() thread" << std::endl;
        perThreadData[i].cv.wait(lock);

        threadData->ok_to_exit = 1;

        // added after comment from Sergey
        threadData->cv.notify_all(); 

        std::cout << "Done - main() thread" << std::endl;
        
    }

    for(int i{0}; i < totalThreadCount; ++i)
    {
        std::size_t thread_index = i;
        ThreadStruct& threadData = perThreadData[thread_index];
        
        std::unique_lock<std::mutex> lock(threadData.m);
        std::cout << "i=" << i << std::endl;
        int &exit_confirm = threadData.exit_confirm;
        threadData.cv.wait(lock, [exit_confirm]{return exit_confirm == 1;});
        std::cout << "i=" << i << " finished!" << std::endl;
    }

    Pause();

    return 0;
}

This runs to the line:

WAIT: Detached thread 0

but the detached thread never quits. What have I done wrong?

Edit: Further experimentation - is this helpful?

I thought it might be helpful to simplify things by removing a step. In the example below, main() does not signal to the detached thread, it just waits for a signal from the detached thread.

But again, this code hangs - after printing DROP... This means the detached thread exits ok, but main() doesn't know about it.

#include <cstdlib> // std::atoi
#include <iostream>
#include <thread>
#include <vector>
#include <random>
#include <future>
#include <condition_variable>
#include <mutex>

struct ThreadStruct
{

    int id;

    std::condition_variable cv;
    std::mutex m;

    int ok_to_exit;
    int exit_confirm;

};



void Pause()
{
    std::cout << "Press enter to continue" << std::endl;
    std::cin.get();
}


void detachedThread(ThreadStruct* threadData)
{
    std::cout << "START: Detached Thread " << threadData->id << std::endl;
    
    // Performs some arbitrary amount of work.
    for(int i = 0; i < 100000; ++ i);

    std::cout << "FINISH: Detached thread " << threadData->id << std::endl;
    
    std::unique_lock<std::mutex> lock(threadData->m);
    
    std::cout << "EXIT: Detached thread " << threadData->id << std::endl;
    threadData->exit_confirm = 1;

    threadData->cv.notify_all();
    std::cout << "DROP" << std::endl;

}

int main(int argc, char** argv)
{
    
    int totalThreadCount = 1;

    ThreadStruct* perThreadData = new ThreadStruct[totalThreadCount];
    std::cout << "Main thread starting " << totalThreadCount << " thread(s)" << std::endl;

    for(int i = totalThreadCount - 1; i >= 0; --i)
    {
        perThreadData[i].id = i;
        perThreadData[i].ok_to_exit = 0;
        perThreadData[i].exit_confirm = 0;
                
        std::thread t(detachedThread, &perThreadData[i]);
        t.detach();
            
    }

    for(int i{0}; i < totalThreadCount; ++i)
    {
        std::size_t thread_index = i;
        ThreadStruct& threadData = perThreadData[thread_index];
        
        std::cout << "Waiting for mutex" << std::endl;
        std::unique_lock<std::mutex> lock(threadData.m);
        std::cout << "i=" << i << std::endl;
        int &exit_confirm = threadData.exit_confirm;
        threadData.cv.wait(lock, [exit_confirm]{return exit_confirm == 1;});
        std::cout << "i=" << i << " finished!" << std::endl;
    }

    Pause();

    return 0;
}

Solution

  • For future interest: fixed the origional MWE posted in the question. There was two issues

    • not capturing local variable in lambda by reference (see other answer)

    • 1 too many wait() calls

        #include <cstdlib> // std::atoi
        #include <iostream>
        #include <thread>
        #include <vector>
        #include <random>
        #include <future>
        #include <condition_variable>
        #include <mutex>
      
        struct ThreadStruct
        {
      
            int id;
      
            std::condition_variable cv;
            std::mutex m;
      
            int ok_to_exit;
            int exit_confirm;
      
        };
      
      
      
        void Pause()
        {
            std::cout << "Press enter to continue" << std::endl;
            std::cin.get();
        }
      
      
        void detachedThread(ThreadStruct* threadData)
        {
            std::cout << "START: Detached Thread " << threadData->id << std::endl;
      
            // Performs some arbitrary amount of work.
            for (int i = 0; i < 100000; ++i);
      
            std::cout << "FINISH: Detached thread " << threadData->id << std::endl;
      
            std::unique_lock<std::mutex> lock(threadData->m);
            std::cout << "WAIT: Detached thread " << threadData->id << std::endl;
            threadData->cv.wait(lock, [&threadData]{return threadData->ok_to_exit == 1;});
            std::cout << "EXIT: Detached thread " << threadData->id << std::endl;
            threadData->exit_confirm = 1;
      
            threadData->cv.notify_all();
            std::cout << "DROP" << std::endl;
      
        }
      
        int main(int argc, char** argv)
        {
      
            int totalThreadCount = 1;
      
            ThreadStruct* perThreadData = new ThreadStruct[totalThreadCount];
            std::cout << "Main thread starting " << totalThreadCount << " thread(s)" << std::endl;
      
            for (int i = totalThreadCount - 1; i >= 0; --i)
            {
                perThreadData[i].id = i;
                perThreadData[i].ok_to_exit = 0;
                perThreadData[i].exit_confirm = 0;
      
                std::thread t(detachedThread, &perThreadData[i]);
                t.detach();
      
            }
      
            for(int i{0}; i < totalThreadCount; ++ i)
            {
                ThreadStruct *threadData = &perThreadData[i];
      
                std::cout << "Waiting for lock - main() thread" << std::endl;
                std::unique_lock<std::mutex> lock(perThreadData[i].m);
                std::cout << "Lock obtained - main() thread" << std::endl;
                //perThreadData[i].cv.wait(lock, [&threadData]{return threadData->ok_to_exit == 1;});
      
                std::cout << "Wait complete" << std::endl;
                threadData->ok_to_exit = 1;
                threadData->cv.notify_all();
                std::cout << "Done - main() thread" << std::endl;
      
            }
      
            for (int i{ 0 }; i < totalThreadCount; ++i)
            {
                std::size_t thread_index = i;
                ThreadStruct& threadData = perThreadData[thread_index];
      
                std::cout << "Waiting for mutex" << std::endl;
                std::unique_lock<std::mutex> lock(threadData.m);
                std::cout << "i=" << i << std::endl;
                int& exit_confirm = threadData.exit_confirm;
                threadData.cv.wait(lock, [&exit_confirm] {return exit_confirm == 1; });
                std::cout << "i=" << i << " finished!" << std::endl;
            }
      
            Pause();
      
            return 0;
        }