Question

This is purely a design philosophy question in the context of C++.

Is it a okay design philosophy to start a thread from within a constructor? I have a library whose sole responsibility is to asynchronously handle some requests by exposed APIs. For this to work it has to start a thread internally.

I have MyClass which is a public class exposed from my library.

//MyClass.h
MyClass {
private:
    std::thread m_member_thread;
}

Is it okay to start the thread from within the constructor like this?

//MyClass.cpp
MyClass::MyClass() {
    // Initialize variables of the class and then start the thread
    m_member_thread = std::thread(&MyClass::ThreadFunction, this);
}

Or, is better to expose a method which the user code of my library explicitly calls to start the thread?

//MyClass.cpp
MyClass::MyClass() {

}

MyClass::StartThread() {
    m_member_thread = std::thread(&MyClass::ThreadFunction, this);
}

Or does it really not matter?

PS:
Yes, the thread is lightweight and does not do much heavy operations. I want to avoid answers which say, it depends on what the thread does etc. This is purely a design philosophy question.

Was it helpful?

Solution

Initializing the object completely in the constructor is usually preferable because it reduces the number of states, and thus making your object easier to reason about. No start and stop methods unless your design really needs this statefulness! Similarly, init methods that have to be called after the constructor are a massive design smell – if you can't initialize an object it shouldn't exist yet, which can be modelled with a null pointer. Stateful operations can also make it impossible to use const properly.

The “constructors should not do real work” advice does not apply very well to C++. Its goal is that running a constructor should be super cheap, similar to C++'s “trivially constructible” concept. Following that advice, any resources should be acquired prior to construction and passed as arguments to the constructor, e.g. with a public static factory member function that calls a private constructor. But this violates RAII (resource acquisition is initialization, not precedes it) and makes it difficult to write exception-safe code. This is not a good approach in C++.

Therefore, it is absolutely correct to create a thread within a constructor (and to throw an exception if something goes wrong).

The standard library includes a couple of examples of related behaviour. File objects such as std::ifstream correctly open/create any files within their constructor. Unfortunately their design is flawed in that the resulting object may not be in a usable state if an error occurred. A better design would have been to throw on errors, or to use a factory function that returns an optional value. Second, consider buffers such as std::vector or std::string. They may allocate memory in the constructor but don't have to. Instead, allocations may be deferred until characters/items are added to the buffer and exhaust its capacity. In your design it could be preferable to defer thread initialization until it is actually used, e.g. until some unit of work is scheduled for the thread.

OTHER TIPS

As general design philosophy it is bad idea. This introduce a race condition.

The new thread starts immediately and may access not yet initialized resources of the object. Additionally, the thread may not use virtual methods. More about this problem please read in this article: https://rafalcieslak.wordpress.com/2014/05/16/c11-stdthreads-managed-by-a-designated-class/

Of course one may argue, that by following some rules it would be ok, but then your code will require "special attention" forever.

That depends on your design philosophy on whether constructors should do real work. If they are allowed to, this is fine, no matter what the thread does. If not, it's not.

Think also about your class's invariants. Is the thread running in the background an invariant of your class? Invariants must be established in the constructor.

Finally, consider this: std::thread's own constructor starts a thread. This may be a hint as to the design philosophy of the standard library designers.

To me the question is, in terms of your needs, how tied the lifetime of your thread is to the lifetime of the object. As a point of consideration, consider a thread pool.

class ThreadPool
{
    ...
private:
    MyClass threads[max_threads];
};

In that case it could become a burden if the lifetime of your objects is tied to the lifetime of your threads, because the thread pool might want to allocate/initialize N threads and corresponding objects in advance and start and stop those threads independently of a ctor and dtor. You can start to fight against the language, especially if performance becomes a concern, rather than working in harmony with the language if your needs actually call for a separation of these two lifetimes (thread lifetime vs. object lifetime) while still trying to stubbornly tie the two together.

You might want to make those two independent of each other if you want to reuse an existing thread resource and start it once more with a new thread state and function to call without actually creating a new thread and instantiating a corresponding new object.

In such a case above, you might want a separate start_thread or start or run sort of method to start running the thread independently of when the wrapper is constructed, as well as a method to terminate the thread prior to the destruction of the object (though it'll probably still be a good idea to ensure termination of the thread in the destruction of the object).

So that's the primary concern I'd consider in this particular case. It would definitely simplify things if you can tie the lifetime of an object to the lifetime of the thread. Yet it might open up more design options if you don't and separate the two. There's sometimes a merit to separating "starting an operation" from "constructing an object"... or not and that could simply translate to more code than should be ideally required if you never actually need that distinction.

Licensed under: CC-BY-SA with attribution
scroll top