start/stop thread in ctor/dtor or better use start()/stop()?

Question

I've a class that internally uses a worker thread. Currently the ctor starts the thread and the dtor stops (and waits for) it. Is this considered good code? I think it would be better to have separate start() / stop() functions for this purpose.

One of the problems is that stopping and waiting for the thread may throw exceptions, which is bad in a dtor.

What would you advice me:

1. leave the code as it is and just catch and log exceptions in the dtor
2. use start() / stop(), let the client handle exceptions and just delete the thread in the dtor (and issue a warning on unclean shutdown or something)

Answer 1

Depends a bit on the semantics of the class. If it needs a thread internally, it's good to start/stop the thread without intervention from the outside.

A good thing might be to use a thread pool, i.e. reuse threads dropped by a previous instance of the class.

Answer 2

I would probably not start the thread in the constructor, but rather have a start function. If the worker thread is basically invisible to the user then it might make little difference, and starting in the constructor might be better. But if the user interacts with the worker thread in any way (e.g. their code is run in it), then eventually someone will need to set up some state after the object is created, but before the thread runs. Murphy's Law guarantees it ;-)

I would stop it in the destructor, and catch and log the exception. If the user is likely to need to know the result of stopping (for instance if failure means that the worker thread might not have done its work), then also have a stop function which they can optionally call in order to get that information.

Btw, there are also some technical concerns to do with starting a thread in a constructor at all. The new thread will potentially run before the constructor returns in the old thread. If it accesses its owner object (for example to report the result), it can therefore access an incompletely-constructed object. That's usually easy to work around, right up to the point where you inherit from your original class. Now the thread is running before the subclass is constructed, which could cause all kinds of trouble. So if you do start a thread in a constructor, and the thread can access the object directly or indirectly, take care and leave lots of warnings.

Answer 3

I don't see both options as mutually exclusive. This is how I'd do it:

{
     mythread worker1; // starts
     mythread worker2(false); // doesn't start
     worker2.start();

     worker1.stop(); 

} // dtor. of worker2 stops it, dtor. of worker1 does nothing