Why thread keeps hanging during condition_variable.wait()

Question

I'm trying to write a program, that uses threads. The thread should sleep and wait until head pointer of Stack changes, then do some stuff and sleep again. However, my thread keeps hanging on wait function, and my program hangs with it, waiting for statement to change. But it won't, because whole program waiting to thread complete. So.. Here is my code, it's working, when I put the thread join into destructor, but I want it to run before the pushes happening, so it can notice changes during run, and then when int main() completed, safely terminated.

#include <iostream>
#include <fstream>

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

struct Node {
    int data;
    Node* next;
};

class Stack {
private:
    Node* head;
    std::mutex mtx;
    std::thread writeThread;
    std::ofstream outFile;
    Node* prevHead;
    std::condition_variable cv;

public:

    Stack() {
        head = nullptr;
        prevHead = nullptr;
        outFile.open("C:\\Users\\mayday\\Desktop\\stack.txt");
        writeThread = std::thread(&Stack::WriteStack, this);
    }


    ~Stack() {
        writeThread.join();
    }

    void Push(int data) {
        std::unique_lock<std::mutex> lock(mtx);
        Node* newNode = new Node();
        newNode->data = data;
        newNode->next = head;
        head = newNode;
        lock.unlock();
        cv.notify_one();
    }

    void WriteStack() {
        while (true) {
            std::unique_lock<std::mutex> lock(mtx);
            cv.wait(lock, [this]() { return head != prevHead; });
            Node* temp = head;
            while (temp != nullptr) {
                outFile << temp->data << " ";
                temp = temp->next;
            }
            outFile << std::endl;
            outFile.close();
            prevHead = head;
            lock.unlock();
        }
    }

};

int main() {
    Stack s;
    s.Push(1);
    s.Push(2);
    s.Push(3);
    return 0;
}

I tried creating new void method for thread join() to execute it from main, but if I implement it before Push(), then it will sleep forever and do nothing. Even with calls from destructor, it's just doing stuff and keeps freezing waiting on condition changes. Of course I can provide it a way out by checking head == prevHead in the end, and then breaking it, but I want it just to sleep in background, until it will be notified for changes. How can I overcome this freeze? Sorry for my bad english.

Answer 1

A few things:

As I called out the comments, there's no reason to explicitly invoke lock.unlock() since the destructor of unique_lock will do that for you.

Also, I tend to favor notify_all() instead of notify_one() just so I don't have to think about what additional edge cases that might create.

But neither of the above issues are the reason for your hang.

Your code is hanging because there's nothing to signal to the thread to exit. Your main thread just hangs on a call to writeThread.join() in the Stack destructor waiting for the worker thread to exit. And your worker thread is just hanging out waiting to be notified again.

Add a new bool member to your class called needToExit initialized to false in the constructor.

In the destructor, set this variable to true (while under a mutex lock) and then signal the cv.

In your thread, check this variable as an exit condition.

Update. Based on your comments, I also added a change to Push such that it will wait for the worker thread to finish processing a previous head pointer change. A cv.notify_all call is made from the thread when to signal back to the main thread each time it finishes processing a head change.

class Stack {
private:
    Node* head;
    std::mutex mtx;
    std::thread writeThread;
    std::ofstream outFile;
    Node* prevHead;
    std::condition_variable cv;
    bool needToExit;   // DECLARE THIS

public:

    Stack() {
        head = nullptr;
        prevHead = nullptr;

        needToExit = false;   // INIT TO FALSE

        outFile.open("C:\\Users\\jselbie\\Desktop\\stack.txt");
        writeThread = std::thread(&Stack::WriteStack, this);
    }


    ~Stack() {

        {
            std::unique_lock<std::mutex> lock(mtx);
            needToExit = true;  // SET UNDER LOCK IN DESTRUCTOR
        }
        cv.notify_all(); // signal cv to wake up thread to check condition

        writeThread.join();
    }


    void Push(int data) {
        std::cout << "push(" << data << ")\n";

        // wait for any previous head change to
        // be picked up and processed by the thread
        cv.wait(lock, [this]() { 
            return (head == prevHead)
        });

        std::unique_lock<std::mutex> lock(mtx);
        Node* newNode = new Node();
        newNode->data = data;
        newNode->next = head;
        head = newNode;
        cv.notify_all();
    }

    void WriteStack() {
        while (true) {
            std::unique_lock<std::mutex> lock(mtx);
            cv.wait(lock, [this]() { 
                // CHECK needToExit in addition to pointer change
                return ((head != prevHead) || needToExit); 
            });

            if (head != prevHead) {
                Node* temp = head;
                while (temp != nullptr) {
                    outFile << temp->data << " ";
                    temp = temp->next;
                }
                outFile << std::endl;
                outFile.close();
                prevHead = head;

                cv.notify_all(); // notify main thread
            }

            if (needToExit)
                break; // EXIT THREAD WHEN needtoExit==true
            }
        }
    }

};