Threads are not being detached

Question

I am trying to make a thread pool. I have an std::unordered_map which maps thread ids to std::threads (workers). The thread ids are the ids of the threads waiting for a task to be pushed into the thread pool (waiters). The tasks are represented by a std::stack (tasks). Once a task is pushed into the pool, the task is popped from the stack and made into a thread function as the "value" part of the map.

In the destructor I try to detach all the threads that are still running. But I still get the following exception:

terminate called without an active exception
bash: line 7: 16881 Aborted                 (core dumped) ./a.out

This means the threads weren't detached and the program terminated. But my destructor goes through the elements and detaches them (I believe). Why is this happening and how can I fix it?

#include <queue>
#include <stack>
#include <mutex>
#include <thread>
#include <algorithm>
#include <functional>
#include <type_traits>
#include <unordered_map>
#include <condition_variable>

template <class F>
class thread_pool
{
    static_assert(std::is_function<F>::value, "F must have function type");
public:
    thread_pool();
    ~thread_pool();
    template <class Task>
    void push(Task&&);
private:
    std::unordered_map<std::thread::id, std::thread> workers;
    std::queue<std::thread> waiters;
    std::stack<std::function<F>> tasks;
    static std::size_t max;
private:
    std::condition_variable m_cond;
    std::mutex m;
private:
    void wait_for_tasks();
};

template <class F>
std::size_t thread_pool<F>::max(10);

template <class F>
thread_pool<F>::thread_pool()
{
    std::lock_guard<std::mutex> lock(m);
    for (std::size_t i = 0; i < max; ++i)
        waiters.emplace(&thread_pool<F>::wait_for_tasks, this);
}

template <class F>
void thread_pool<F>::wait_for_tasks()
{
    while (true)
    {
        std::unique_lock<std::mutex> lock(m);
        m_cond.wait(lock, [this] { return !tasks.empty(); });

        auto f = tasks.top();
        tasks.pop();
        auto& th = workers[std::this_thread::get_id()];

        if (th.get_id() == std::thread::id())
            th = std::thread(f);
    }
}

template <class F>
template <class Task>
void thread_pool<F>::push(Task&& t)
{
    {
        std::lock_guard<std::mutex> lock(m);
        tasks.emplace(std::forward<Task>(t));
    }
    m_cond.notify_all();
}

template <class F>
thread_pool<F>::~thread_pool()
{
    std::for_each(workers.begin(), workers.end(), [] (std::pair<std::thread::id const, std::thread>& p)
    {
        if (p.second.joinable())
            p.second.detach();
    });

    while (!waiters.empty())
    {
        auto& t = waiters.front();
        waiters.pop();
        if (t.joinable())
            t.detach();
    }
}

int main()
{
    thread_pool<void ()> pool;
}

I'm not even sure this is the best way to do it, this is my first time making one. Here is a demo.

Answer 1

In your pool destructor, you are calling pop before detaching the thread, effectively destructing a thread that is joinable : the standard guarantees that this will call std::terminate

First detach the thread, and then pop it out of the queue :

while (!waiters.empty())
{
    auto& t = waiters.front();

    if (t.joinable())
        t.detach();

    waiters.pop();
}