2

I have a state machine being processed within a std::thread. This state machine initializes a network connection, processes data, and upon the receipt of a certain message, needs to shut itself down. Using join in this fashion triggers the 'abort() has been called' exception. Is this one of the cases where a detached thread is appropriate.

#include <iostream>

#include <thread>
#include <atomic>
#include <memory>

class ThreadExample
{
public:
    ThreadExample()
    {
        StartThread();
    }

    void StartThread()
    {
        //start thread;
        run_thread = true;
        the_thread = std::thread(&ThreadExample::ThreadFunction, this);
    }

    void ThreadFunction()
    {
        while (run_thread)
        {
            if (correct_message_found)
                ShutdownThread();
            else
                ProcessMessage();   //example code to imitate network processing

            //arbitrary wait. not relevant to the problem
            std::this_thread::sleep_for(std::chrono::seconds(1));
        }
    }

    //read in by some network connection
    void ProcessMessage(/*some message data*/)
    {
        static int counter = 0;
        if (counter == 3)
        {
            correct_message_found = true;
        }
        else
        {
            std::cout << "Waiting for the right message\n";
            counter++;
        }
    }

    void ShutdownThread()
    {
        run_thread = false;
        if (the_thread.joinable())
            the_thread.join();
    }

private:
    std::thread the_thread;
    std::atomic_bool run_thread;
    bool correct_message_found = false;
};

int main()
{
    auto example = std::make_unique<ThreadExample>();

    int data;
    std::cin >> data;
}
Jason
  • 2,147
  • 6
  • 32
  • 40
  • 2
    Sorry, but the description is not clear enough. How does the thread shut itself down? Who calls `join` on what? Where would you be detaching the thread? What are the side effects of the thread and why is it threaded in the first place? Please do create the minimal representative example you are talking about, it will be much clearer. And what exactly is the question you would like answered? – Max Langhof Aug 23 '19 at 11:19
  • I'll mock up something. – Jason Aug 23 '19 at 11:22
  • 2
    You can still exit from the thread any time you want (by returning from the threads main function), as long as someone *else* remembers to `join` the thread. – Some programmer dude Aug 23 '19 at 11:23
  • But of course you could also detach the thread, but then you should add some logic to communicate process termination to the thread (if it's still running then). – Some programmer dude Aug 23 '19 at 11:24
  • @Someprogrammerdude - I would imagine that the destructor is still verboten as a place to call join()? – Jason Aug 23 '19 at 11:25
  • 2
    You can't join a thread from itself. It has to be done by another thread. But it's not forbidden or prohibited to do it from a destructor, as long as it's not the thread itself that destructs the object. – Some programmer dude Aug 23 '19 at 11:25
  • @Someprogrammerdude - that is what i had originally. I detached and ran. When the message was received to indicate that i was done with what i need, i set my poison pill variable which ended the thread loop. – Jason Aug 23 '19 at 11:26
  • you question is rather unclear. In some sense it is always the thread itself that decides when to terminate. You need to do something extra (condition variables et al) to control termination from within a different thread – 463035818_is_not_an_ai Aug 23 '19 at 11:35
  • 3
    Your first two sentences seem rather unconnected. "Using join in this fashion ..." in what fashion? calling `join` does not make a thread terminate, it just makes the calling thread wait until the other finishes its execution – 463035818_is_not_an_ai Aug 23 '19 at 11:37
  • detached thread is appropriate only if you are dealing with some old broken code that prevents thread from finishing gracefully – user7860670 Aug 23 '19 at 11:45
  • In conslusion to @formerlyknownas_463035818 's comment: Even if you *could* call join from within same thread, this would result in deadlock, the thread being waiting (still active!) for terminating itself... This is prevented by [join](https://en.cppreference.com/w/cpp/thread/thread/join) (see error conditions), though. – Aconcagua Aug 23 '19 at 11:45
  • Updated with source. Take it easy on me! – Jason Aug 23 '19 at 11:57
  • @Aconcagua Well that's exactly what's being done! – Max Langhof Aug 23 '19 at 11:58
  • @MaxLanghof Sure, I see – code just wasn't there when I posted my comment... – Aconcagua Aug 23 '19 at 12:00
  • @Aconcagua Yes, I didn't mean to imply you should've known, just wanted to let you know you are correct. Sorry, poor wording on my part. – Max Langhof Aug 23 '19 at 12:02
  • @MaxLanghof Never mind ;) – Aconcagua Aug 23 '19 at 12:03
  • @VTT, calling `detach()` is appropriate for a thread that _never_ terminates. Some people--mostly people who implement cloud service-y stuff--advocate for "crash-only code." That is, systems that have no concept of a graceful shutdown. If, for some highly unusual reason, you need the service to stop, then you just pull the plug. They argue that, since the plug could be pulled by accident anyway, then it should be sufficiently robust that pulling the plug causes no lasting harm. – Solomon Slow Aug 23 '19 at 13:58
  • @SolomonSlow Programs that are not required to perform clean shutdown are not that uncommon. For example all those new UWP apps MS is pushing. Even if code is supposed to be used inside of such a program it does not make use of `detach()` appropriate. The same code will be used in development environment or may be reused later in some other context where performing clean exit is mandatory. And the thing is that the code performing proper cleanup actions will work perfectly fine even if cleanup code is never executed at runtime, while code – user7860670 Aug 24 '19 at 10:12
  • with `detach()` will be completely broken when cleanup is required. Note that use of `detach` often also hides problems in underlaying code that can not be resolved by just switching to `join`. – user7860670 Aug 24 '19 at 10:12

1 Answers1

4

The correct way to terminate a thread from inside itself is to simply return from the function the thread is executing:

void ThreadFunction()
{
    while (run_thread)
    {
        if (correct_message_found)
            return;
        else
            ProcessMessage();   //example code to imitate network processing

        //arbitrary wait. not relevant to the problem
        std::this_thread::sleep_for(std::chrono::seconds(1));
    }
}

Calling join from within the thread that is supposed to be joined is an error, see the first error condition: https://en.cppreference.com/w/cpp/thread/thread/join

join means "wait for the given thread to finish, then continue on". You are telling a thread to wait until it itself is finished. So it can only end once it has already ended, which is clearly contradictory.

Where you should call join is in the destructor of ThreadExample. ThreadFunction uses members of ThreadExample, and ThreadExample also owns the std::thread object, so ThreadExample cannot be allowed to die while the thread is still running. In the code you show, you would run into that problem if you input something before the thread is done: ThreadExample is then destroyed, and with it the std::thread object living inside. If a std::thread is destroyed while joinable (i.e. with a non-detached thread still running) then std::terminate is called:
https://en.cppreference.com/w/cpp/thread/thread/%7Ethread

Max Langhof
  • 23,383
  • 5
  • 39
  • 72
  • Thanks. I had read somewhere that it was not advised to call join in the destructor so i was avoiding that. for an example like this, is there a more appropriate pattern? – Jason Aug 23 '19 at 12:04
  • I'd yet add the `main` function to show where to indeed call `join`. – Aconcagua Aug 23 '19 at 12:05
  • @Jason Where is that advice from? It is correct that you want to avoid throwing exceptions in destructors (which `std::thread::join` [might do](https://en.cppreference.com/w/cpp/thread/thread/join)), but you can catch that one. – Max Langhof Aug 23 '19 at 12:16
  • @MaxLanghof - https://stackoverflow.com/questions/22803600/when-should-i-use-stdthreaddetach – Jason Aug 23 '19 at 12:46
  • @Jason I don't see how this relates to calling `join` in a destructor, except for the exact advice I provided: Destroying a `std::thread` that has neither been joined nor detached is bad. – Max Langhof Aug 23 '19 at 12:49
  • @MaxLanghof - i was basing it on this statement: Thus, you should always either join or detach a thread before the flows of execution reaches the destructor. – Jason Aug 23 '19 at 12:56
  • @Jason Yes, before it reaches the destructor of `std::thread`, not before it reaches _any_ destructor. Within the destructor of `ThreadExample`, `the_thread` is still alive - its destructor is called _after_ you return from `ThreadExample::~ThreadExample`. – Max Langhof Aug 23 '19 at 13:01
  • @MaxLanghof - That makes sense. Thanks for the input and patience! – Jason Aug 23 '19 at 13:05