Should I delete a copy constructor and an assignment operator of a class that internally creates threads?

Question

When a class owns thread and mutex objects, are there any potential dangerous situations where a copy/assignment would be dangerous, meaning that copy constructor and assignment should be deleted?

Consider this sample code:

class A : B
{
    std::thread t;
    std::mutex m;

  public:
    A() : B() {}
    virtual ~A()
    {
        if (t.joinable())
            t.join();
    }

    // Should I delete cctor and assignment operator?

    virtual void Method()
    {
        t = std::thread([this] 
        {
            std::lock_guard<std::mutex> lck(m);
            ... // processing
        });
    }

    // Other methods that lock on mutex m
};

If I understand things correctly, the thread created in Method() will not be visible outside of A, meaning that copying with a default cctor shouldn't be problematic, because the entire state would get copied. Is my reasoning correct?

Answer 1

Any class whose (extended) state includes pointers into itself must have a deleted copy/move or must marshall that state.

t = std::thread([this] 

the above line stores a pointer to this in the class's extended state.

Default copy and move is thus inappropriate. Deleted is one option; carefully and possibly expensively marshalling another.

In addition, both thread and mutex are move only types. So copy will be implicitly deleted.

Your move assign/construct, however, will be written by the compiler and wrong. So delete them or fix them.

---

There is a idiom from languages without class-value types (like Java/C#) of having a class with state and a thread that works on the state. This is a bad plan in a value centric language like C++. Store your state externally (say, a shared or unique ptr), share it with your thread (as shared ptr or as observing ptr), and suddenly default move makes sense.

Without doing that, your object becomes sessile -- unable to safely move -- which cripples a lot of great C++ idioms, or forces external smart pointer wrapping.

Answer 2

Both copy constructor and assign need to be deleted or defined. If you want to delete them, don't do anything: C++ will delete them for you implicitly.

Your class has std::mutex as a member. Its copy constructor and assignment operators are deleted. This makes C++ delete copy constructor and assignment operator of your class as well, because generating default behavior for them would require invoking deleted members.

Note: Deletion is not your only option: if you find it useful to make copies of A or make them assignable, you have an option to define a copy constructor and an assignment operator that create a new thread and a new mutex, and copy relevant parts of your object to support the semantics that you find useful. Even though C++ wouldn't do it for you by default, you have an option to do it manually.