Santus Westil
Santus Westil

Reputation: 43

Thread-safe stack mutex destroyed while busy

I've been learning C++ standard library multithreading for a while and as an exercise I wanted to make a thread-safe stack using mutex blocking and condition variables. Here's the class I made:

#ifndef THREADSAFE_STACK_H_
#define THREADSAFE_STACK_H_
#include <mutex>
#include <condition_variable>
#include <stack>
#include <memory>


template <typename T> class threadsafe_stack
{
    public:
        mutable std::mutex mut;
        std::stack<T> st;
        std::condition_variable cond;

    public:
        threadsafe_stack() = default;
        threadsafe_stack(const threadsafe_stack &s);
        threadsafe_stack(threadsafe_stack &&s);
        threadsafe_stack& operator=(const threadsafe_stack &s);
        threadsafe_stack& operator=(threadsafe_stack &&s);

        void push(const T &t);
        void push(T &&t);

        bool try_pop(T &t);
        std::shared_ptr<T> try_pop();

        void wait_and_pop(T &t);
        std::shared_ptr<T> wait_and_pop();

        typename std::stack<T>::size_type size() const;
        bool empty() const;
};


template <typename T> threadsafe_stack<T>::threadsafe_stack(const threadsafe_stack &s)
{
    std::lock(mut, s.mut);
    std::lock_guard<std::mutex> lock1(mut, std::adopt_lock);
    std::lock_guard<std::mutex> lock2(s.mut, std::adopt_lock);
    st = s.st;
}


template <typename T> threadsafe_stack<T>::threadsafe_stack(threadsafe_stack &&s)
{
    std::lock(mut, s.mut);
    std::lock_guard<std::mutex> lock1(mut, std::adopt_lock);
    std::lock_guard<std::mutex> lock2(s.mut, std::adopt_lock);
    st = std::move(s.st);
}


template <typename T> threadsafe_stack<T>& threadsafe_stack<T>::operator=(const threadsafe_stack &s)
{
    if (this == &s)
        return *this;

    std::lock(mut, s.mut);
    std::lock_guard<std::mutex> lock1(mut, std::adopt_lock);
    std::lock_guard<std::mutex> lock2(s.mut, std::adopt_lock);
    s = s.st;

    return *this;
}


template <typename T> threadsafe_stack<T>& threadsafe_stack<T>::operator=(threadsafe_stack &&s)
{
    std::lock(mut, s.mut);
    std::lock_guard<std::mutex> lock1(mut, std::adopt_lock);
    std::lock_guard<std::mutex> lock2(s.mut, std::adopt_lock);
    s = std::move(s.st);
}


template <typename T> void threadsafe_stack<T>::push(const T &t)
{
    std::lock_guard<std::mutex> lock(mut);
    st.push(t);
    cond.notify_one();
}


template <typename T> void threadsafe_stack<T>::push(T &&t)
{
    std::lock_guard<std::mutex> lock(mut);
    st.push(std::move(t));
    cond.notify_one();
}


template <typename T> bool threadsafe_stack<T>::try_pop(T &t)
{
    std::lock_guard<std::mutex> lock(mut);
    if (st.empty())
        return false;

    t = st.top();
    st.pop();
    return true;
}


template <typename T> std::shared_ptr<T> threadsafe_stack<T>::try_pop()
{
    std::lock_guard<std::mutex> lock(mut);
    if (st.empty())
        return std::shared_ptr<T>();

    std::shared_ptr<T> result(new T{st.top});
    st.pop();
    return result;
}


template <typename T> void threadsafe_stack<T>::wait_and_pop(T &t)
{
    std::unique_lock<std::mutex> lock(mut);
    cond.wait(lock, [this]{ return !st.empty(); });

    t = st.top();
    st.pop();
}


template <typename T> std::shared_ptr<T> threadsafe_stack<T>::wait_and_pop()
{
    std::unique_lock<std::mutex> lock(mut);
    cond.wait(lock, [this]{ return !st.empty(); });

    std::shared_ptr<T> result(new T{ st.top });
    st.pop();
    return result;
}


template <typename T> typename std::stack<T>::size_type threadsafe_stack<T>::size() const
{
    std::lock_guard<std::mutex> lock(mut);
    return st.size();
}


template <typename T> bool threadsafe_stack<T>::empty() const
{
    std::lock_guard<std::mutex> lock(mut);
    return st.empty();
}


#endif

Then I wanted to test it in a simple program like this:

#include <iostream>
#include <condition_variable>
#include <ctime>
#include <chrono>
#include <thread>
#include "threadsafe_stack.h"


threadsafe_stack<int> data_stack;
void process_data()
{
    while (true)
    {
        int val = 0;
        data_stack.wait_and_pop(val);
        std::cout << val << " ";
    }
}


int main()
{
    std::thread processor(process_data);
    processor.detach();

    int values[10] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
    for (int i = 0; i < 10; i++)
    {
        std::this_thread::sleep_for(std::chrono::milliseconds(100));
        data_stack.push(values[i]);
    }
    std::this_thread::sleep_for(std::chrono::milliseconds(100));

    system("pause");
    return 0;
}

The output is:

1 2 3 4 5 6 7 8 9 10 Press any key to continue . . .
f:\dd\vctools\crt\crtw32\stdcpp\thr\mutex.c(40): mutex destroyed while busy

The second line appears immediately after I press any key or try to close the console. Along with the additional message I get an error (unhandled exception I guess?) It sounds to me like the mutex is being used while I exit the program but it shouldn't since the conditional variable wait should unlock it while waiting. I am clueless.

Upvotes: 2

Views: 3295

Answers (1)

Puppy
Puppy

Reputation: 147028

It occurs because the program exits, which destroys the global variable. If you want to wait until the program has finished working, you need to join the processor thread at the bottom (and not detach it).

The condition variable can only make the other thread sleep, not the main thread.

Upvotes: 1

Related Questions