Is it ok/safe to delete a class that contains running threads when I want the threads to terminate?

Question

Here is a quick example:

class worker
{
    std::thread thread1;
    std::thread thread2;

    worker(){}

    start()
    {
        thread1 = std::thread([]()
        {
            std::this_thread::sleep_for(std::chrono::milliseconds(50000));
        });
        thread1.deteach();

        thread2 = std::thread([]()
        {
            std::this_thread::sleep_for(std::chrono::milliseconds(50000));
        });
        thread2.deteach();
    }

    ~worker()
    {
        // Ignore this part! - as per Richard's comment :)
        //thread1.join();   // <-- do I need this at all?
        //thread2.join();   // <-- do I need this at all?
    }
}

int main()
{
    worker *p_worker = new worker();
    p_worker->start();
    std::this_thread::sleep_for(std::chrono::milliseconds(1000)); // 1 sec

    delete p_worker;
}

• Create the worker
• Start the threads which last for 50 seconds
• After 1 second delete the worker (calls destructor)
• In worker destructor I re-join the threads (probably should check if they are joinable first?) - not really sure I need to do this.
• Then worker is destroyed

I had a look at this question: how-do-i-terminate-a-thread-in-c11 which suggests there is no c11 portable way to terminate the threads.

Questions:

• Are the threads destroyed completely (no dregs/leaks left)?
  • If "yes" do I need to re-join the threads in order for them to be destroyed? EDIT - Richard pointed out this is N/A
• Is this a sensible approach?

Answer 1

As already stated in comments, you cannot join a detached thread. Detached threads are meant to run independently. In general, it is a bad idea to detach a thread owned by a class.

I would suggest using a boolean to control the lifecycle of your thread.

For example, you could do something like this:

class worker
{
private:
    std::thread thread1;
    std::atomic<bool> thread1ShouldRun;
    std::thread thread2;
    std::atomic<bool> thread2ShouldRun;

    void workerFunc1() {
        bool threadWorkIsDone = false;
        while (thread1ShouldRun.load()) {
            // Do Stuff
            // Set threadXShouldRun to false when you're done
            // thread1ShouldRun.store(false);
        }
    }

    void workerFunc2() {
        bool threadWorkIsDone = false;
        while (thread2ShouldRun.load()) {
            // Do Stuff
            // Set threadXShouldRun to false when you're done
            // thread2ShouldRun.store(false);
        }
    }

public:
    worker() {}

    void start()
    {
        thread1ShouldRun.store(true);
        thread1 = std::thread(&worker::workerFunc1, this);
        thread2ShouldRun.store(true);
        thread2 = std::thread(&worker::workerFunc2, this);            
    }

    ~worker()
    {
        thread1ShouldRun.store(false);
        // Protection in case you create a worker that you delete and never call start()
        if (thread1.joinable())
            thread1.join();
        thread2ShouldRun.store(false);
        if (thread2.joinable())
            thread2.join();
    }
};

int main()
{
    worker *p_worker = new worker();
    p_worker->start();
    std::this_thread::sleep_for(std::chrono::milliseconds(1000)); // 1 sec

    delete p_worker; // Threads will be joined here
}

Answer 2

Yes, the threads are completely destroyed by the std::thread dtor if they are still joinable (running and not detached).

That's not good news though, as std::terminate() will be called, killing the whole process.

In general, just terminating is only sensible to avoid further damage from an unexpected state, or if the application was built for harmless termination at that exact point.