How to fix `terminate called without an active exception` when calling join

Question

I've written a simple implementation of thread pool and I get terminate called without an active exception error. I've checked if threads are joinable then I tried to debug where destructors are called but to no success.

workerpool.hpp

#ifndef WORKERPOOL_H
#define WORKERPOOL_H

#include <condition_variable>
#include <iostream>
#include <memory>
#include <mutex>
#include <optional>
#include <queue>
#include <thread>
#include <vector>

class task {
  public:
    virtual void operator()() = 0;
    virtual ~task() = default;
};

class WorkerPool {
  private:
    class Worker {
      private:
        std::shared_ptr<WorkerPool> wp;
        long id;

      public:
        Worker(std::shared_ptr<WorkerPool> _wp, long _id) : wp(_wp), id(_id) {
            std::cout << "Worker " << id << " created" << std::endl;
        };
        ~Worker() { std::cout << "Worker " << id << " destroyed" << std::endl; }

        void operator()() {
            while (!wp->stop) {
                auto t = wp->fetch_task();
                if (!t.has_value())
                    continue;
                else
                    t.value()->operator()();
            }
            std::cout << "thread " << id << " ended" << std::endl;
        };
    };

    std::vector<std::thread> workers;

    std::queue<std::unique_ptr<task>> q;
    std::condition_variable cv;
    std::mutex mx;

    std::optional<std::unique_ptr<task>> fetch_task() {
        std::unique_lock l(mx);
        cv.wait(l, [&] { return !q.empty() || stop; });
        if (stop)
            return {};
        auto res = std::move(q.front());
        q.pop();
        return std::move(res);
    };

  public:
    WorkerPool() {
        std::cout << "worker pool created" << std::endl;
        for (long i = 0; i < std::thread::hardware_concurrency(); i++) {
            workers.push_back(
                std::thread(Worker(std::shared_ptr<WorkerPool>(this), i)));
        }
    }

    ~WorkerPool() {
        std::cout << "worker pool destroyed" << std::endl;
        terminate();
        for (size_t i = 0; i < workers.capacity(); i++) {
            if (workers[i].joinable())
                workers[i].join();
        }
    }

    WorkerPool(WorkerPool const &) = delete;
    WorkerPool &operator=(WorkerPool const &) = delete;
    WorkerPool(WorkerPool &&) = delete;
    WorkerPool &operator=(WorkerPool &&) = delete;

    bool stop;

    void submit(std::unique_ptr<task> t) {
        std::lock_guard l(mx);
        q.push(std::move(t));
        cv.notify_one();
    }

    void terminate() {
        stop = true;
        cv.notify_all();
    }
};

#endif // WORKERPOOL_H

main.cpp

#include <workerpool.hpp>

#include <condition_variable>
#include <iostream>
#include <mutex>

using namespace std;

class foo : public task {
public:
    void operator()() override { cout << "test" << endl; }
};

int main(int argc, char *argv[]) {

    WorkerPool wp;

    wp.submit(make_unique<foo>());

    wp.terminate();

    cout << "program ended" << endl;
    return 0; 
}

Console output:

> make run
rm -f bin/*
clang++ -std=c++17 -fuse-ld=lld  -pthread -Iinclude -Llib -O2 src/main.cpp -o bin/main 
./bin/main
worker pool created
Worker 0 created
Worker 0 destroyed
Worker 0 destroyed
Worker 1 created
Worker 1 destroyed
Worker 1 destroyed
Worker 2 created
Worker 2 destroyed
Worker 2 destroyed
Worker 3 created
Worker 3 destroyed
Worker 3 destroyed
thread 0 ended
Worker 0 destroyed
worker pool destroyed
program endedthread 2 ended
Worker 2 destroyed
worker pool destroyed

worker pool destroyed
terminate called without an active exception
make: *** [Makefile:32: run] Aborted (core dumped)

Answer 1

std::shared_ptr<WorkerPool>(this)

You can't just do that. You're telling the shared_ptr that it owns the object, but it doesn't. Its lifetime is already being managed elsewhere (in this case, "on the stack" in main).

Consequently you have apparently ended up with a double delete (notice 2 × "worker pool destroyed"). Formally, the results are undefined, particularly as you create yet more unrelated shared_ptrs for each worker thread: if the program hadn't crashed, I wager you'd've seen 4 × that line.

There are ways to make this possible, using enable_shared_from_this, though that cannot work in the constructor (because the "original" shared_ptr won't have finished taking ownership yet).