Is this way of creating detached std::thread with pointer to instance wrong?

Question

I've come across classes whose only function is to continuously do some work in a loop and they are designed such that they define a public method that can be called to invoke this member function in a new std::thread. I'm referring to something like this:

class ThreadLooper {
 public:
  ThreadLooper(const std::string &thread_name)
      : thread_name_{thread_name}, loopCounter_{0} {}

  ~ThreadLooper() {
    cout << thread_name_ << ": destroyed and counter is " << loopCounter_
         << std::endl;
  }

  void run() {
    std::thread([this]() { detachedThreadLoop(); }).detach();
  }

 private:
  void detachedThreadLoop() {
    cout << thread_name_ << ": detachedThreadLoop() started running"
         << std::endl;
    while (true) {
      using namespace std::literals::chrono_literals;
      std::this_thread::sleep_for(2s);
      ++loopCounter_;
      cout << thread_name_ << ": counter is " << loopCounter_ << std::endl;
    }
  }

  std::string thread_name_;
  std::atomic_uint64_t loopCounter_;
};

int main() {
  cout << "In main()" << std::endl;
  {
    ThreadLooper threadLooper{"looper1"};
    threadLooper.run();
    using namespace std::literals::chrono_literals;
    std::this_thread::sleep_for(20s);
    cout << "main() done sleeping, exiting block scope..." << std::endl;
  }

  while (true) {
    using namespace std::literals::chrono_literals;
    std::this_thread::sleep_for(20s);
    cout << "main() woke up..." << std::endl;
  }

  return 0;
}

It seems like because the function running in the detached thread has a pointer to the instance but can continue to run beyond the lifetime of that instance this is bad. I've seen other classes where the thread isn't detached and then in the destructor a flag is set to tell the thread loop to exit and the thread is then joined in the destructor. It seems like the latter is the correct way to do this and that the former relies on the fact that the class will only be used in situations where instances of it live for the duration of the program. Is this correct or am I missing something?

Threads have little to do with the core issue, and you seem to already understand that. The problem is one of the automatic scope of an object terminating, and with that the object itself, leaving any references or pointers to said-same "dangling". Any dereference thereafter invokes UB. The same problem frequently happens without threads by returning a reference or pointer to an automatic from a simple function call, then utilizing it afterward. I.e. Type x, *p = &x; return p; in some Type *foo(). Calling that function, then exercising the result after the object it long gone, invokes UB. — WhozCraig
– WhozCraig, Commented Jan 13, 2019 at 19:56
If you post this as an answer I'll accept it. The involvement of the implicit this here is why I wasn't as sure as with the other examples you provided. — Mike Sweeney
– Mike Sweeney, Commented Jan 14, 2019 at 2:36

Alan Birtles · Accepted Answer · 2019-01-13 19:48:42Z

3

Yes, using std::thread::detach means you need to have your own method of making sure the thread terminates before all the resources it uses are destroyed.

In this case ThreadLooper will invoke undefined behaviour when the program exits the first block scope in main(). It's better to not use detach() then std::thread will call std::terminate if you've forgotten to call join() before the thread (and its containing object) are destroyed.

answered Jan 13, 2019 at 19:48

Alan Birtles

38.3k4 gold badges39 silver badges73 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Solomon Slow Over a year ago

You don't need to "detach" a thread in order for it to use a dangling pointer. You can still make that same mistake in a thread that wants to be join()ed. You just have to work a little harder at making it is all.

Collectives™ on Stack Overflow

Is this way of creating detached std::thread with pointer to instance wrong?

1 Answer 1

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related