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
}