Reconstructing class from member function

Question

I have a small wrapper around std::thread that automatically joins at the end of it's lifetime if possible that looks like this (the real one has more functionality, but the thread is just an example for the problem):

#include <thread>

struct my_thread : std::thread
{
    // Default Constructor
    my_thread() = default;

    // Forwards anything to thread
    template<typename... Ts>
    my_thread(Ts&&... ts)
    :
        std::thread( std::forward<Ts>(ts)... )
    {}

    // Destructor
    ~my_thread()
    {
        if ( joinable() ) { join(); }
    }
};

And from this I wanted to add a restart member function that joins the current thread ( if any ) and creates a new thread. The easiest way I came across was this:

// Restarts the thread
template<typename... Ts>
void restart(Ts&&... ts)
{
    // Destruct current thread
    this->~my_thread();

    // Create new thread on this object
    new (this) my_thread ( std::forward<Ts>(ts)... );
}

But I'm not sure this is a good practice or even if it could fail. The reasons I can think of it for failing is:

• (1) If calling the destructor throws then when the stack is unwound, the destructor will be called again when the local variable is being destructed.

• (2) If calling the constructor throws anything, then the object will be in an incomplete state and when the local variable is destructed the destructor will be called on an incomplete object, which is undefined behavior

Since both the problems involve exceptions I thought I could possible add a try / catch all statement, but I'm unsure on what to do when it fails.

For example, if the destructor succeeds but the constructor fails what should I do?

I can't leave the object in a incomplete state since when the destructor of the local variable is called it will be undefined behavior, so I thought of just doing an infinite loop until the constructor succeeds, but this doesn't sound like a good idea.

Should I save the current object somewhere and, if either fail, revert back like this?

// Restarts the thread
template<typename... Ts>
void restart(Ts&&... ts)
{
    // Copy
    char backup[ sizeof( my_thread ) ];
    memcpy(backup, this, sizeof(my_thread) );

    try
    {
        // Destruct current thread
        this->~my_thread();

        // Create new thread on this object
        new (this) my_thread ( std::forward<Ts>(ts)... );
    }
    catch (...)
    {
        // Copy backup
        memcpy(this, backup, sizeof(my_thread));
    }
}

This feels like an awful idea because not only am I creating unnecessary copies when there are no exceptions I'm also ignoring any side-effects from the constructor/destructor, so I don't believe this to be a good answer.

I also thought of just using std::thread's move constructor for this like so:

// Restarts the thread
template<typename... Ts>
void restart(Ts&&... ts)
{
    static_cast<std::thread&>(*this) = std::move( std::thread{ std::forward<Ts>(ts)... } );
}

It seems to work, but I'm not sure of the pitfalls of this one. And I believe the top one is more useful in general since it also works for types that aren't movable nor copyable, so I'd like to know what the best way to do it is.

In concrete, my question is if it is a good idea to "reconstruct" an object like this and is there anything that should be taken into consideration while doing it?

Also this can apply to anything, I just used threads because this is where the problem first came up.

Edit

Here's an example of what I'd like the end product to do, but if possible without stopping inheriting from std::thread (or any type):

struct my_thread
{
    // The thread memory
    char thread_mem[ sizeof(std::thread) ];

    bool thread_constructed = false;

    // Default Constructor
    my_thread() = default;

    // Forwards anything to thread
    template<typename... Ts>
    my_thread(Ts&&... ts)
    {
        new ( thread_mem ) std::thread ( std::forward<Ts>(ts)... );
        thread_constructed = true;
    }

    // Destructor
    ~my_thread()
    {
        // Call destructor if it's constructed
        if ( thread_constructed )
        {
            if ( reinterpret_cast<std::thread*>(thread_mem)->joinable() )
            {
                reinterpret_cast<std::thread*>(thread_mem)->join();
            }

            thread_constructed = false;
            reinterpret_cast<std::thread*>(thread_mem)->~thread();
        }
    }

    // Restarts the thread
    template<typename... Ts>
    void restart(Ts&&... ts)
    {
        // Destruct current thread
        this->~my_thread();

        // Create new thread on this object
        new (this) my_thread ( std::forward<Ts>(ts)... );
    }
};

Edit 2

I was hoping that it would be possible to make a 100% generic type that could handle this, I believe the following does the double destructor calls problem from before as well as the incomplete object being destructed problem:

template<typename T>
struct restartable_type
{
    // Memory
    alignas(T) char mem[ sizeof(T) ];

    bool constructed = false;

    template<typename... Ts>
    restartable_type(Ts&&... ts)
    {
        new ( mem ) T ( std::forward<Ts>(ts)... );
        constructed = true;
    }

    ~restartable_type()
    {
        reinterpret_cast<T*>(mem)->~T();
        constructed = false;
    }

    template<typename... Ts>
    void restart(Ts&&... ts)
    {
        this->~restartable_type();

        new ( this ) restartable_type<T> ( std::forward<Ts>(ts)... );
    }
};

The only thing I'd like to improve on it would be to inherit from T, so that I could call member functions from T on a restartable_type, but I am not aware of a way to solve the destructor problem with this, all I could think of was to overload operator-> to redirect to T, but this would just be a half-solution since I'd prefer to use . (dot), I'm just not aware of how to solve the destructor problem, and that's what I'd like help with. I apologize if I wasn't clear on the problem.

Answer 1

It is possible to make this idea work when you can fall back to a noexcept default constructor if your main constructor throws:

template<typename... Ts>
void restart(Ts&&... ts)
{
    this->~my_thread();

    try {
        new (this) my_thread(std::forward<Ts>(ts)...);
    } catch (...) {
        new (this) my_thread();
        throw;
    }
}

You can even enable it at compile time by examining is_nothrow_default_constructible<std::thread>::value, which shall be true.

However, there is probably too much that one needs to take into consideration while doing it. For example, no one can inherit from your class further. Or you may be implicitly exposing some methods of the interface for which the class contract can be broken on reconstruction. With std::thread, it's get_id() and native_handle() - they are also broken on assignment, though, so it's not a big problem if your user knows that restart() is an assignment. What might be worse is the ambiguity that arises when the user calls detach() and then restart(). It will work, but not in the sense that the user likely expects.

And one more thing to consider: you don't stop threads by joining them. Joining just waits until the thread finishes. You need some other signalling mechanism to tell the thread to finish, and then you don't gain much with using such a wrapper.

Answer 2

There are some ways of making a general solution, each with their ups and downs:

1 - std::optional<T>

By having a member of std::optional<T> you can provide a thin owning wrapper around T as such:

#include <optional>

template<typename T>
struct reset_type
{
    // The value itself
    std::optional<T> value;

    // Constructs value in-place with arguments
    template<typename... Ts>
    constexpr reset_type(Ts&&... ts)
    :
        value(std::in_place_t{}, std::forward<Ts>(ts)... )
    {}

    // Resets the type
    template<typename... Ts>
    constexpr void reset(Ts&&... ts)
    {
        value.reset();
        value.emplace( std::forward<Ts>(ts)... );
    }

    // Conversion operator
    constexpr operator       T&()       { return *value; }
    constexpr operator const T&() const { return *value; }

    // Accesses T
    constexpr       T* operator->()       { return &(*value); }
    constexpr const T* operator->() const { return &(*value); }
};

This way is most likely the easiest and best way to achieve this wrapper, the only thing you have you give up is accessing members via the . (dot) operator.

2 - Inheritance

We can inherit from T and construct it that way as such:

template<typename T>
struct reset_type : T
{
    // Constructs value in-place with arguments
    template<typename... Ts>
    constexpr reset_type(Ts&&... ts)
    :
        T( std::forward<Ts>(ts)... )
    {}

    
    // Resets the type
    template<typename... Ts>
    constexpr void reset(Ts&&... ts)
    {
        this->~reset_type();

        new ( this ) reset_type<T> ( std::forward<Ts>(ts)... );
    }
};

Problems with this arise from when the constructor or destructor throw an exception, I'm not sure what the details of it are, but unless they throw an exception nothing bad will happen

Upsides with this is that you can convert to T with a static cast, since it inherits from it and that you can access T with the . (dot) operator, making this much more useful.

Might also be worth it to replace a few T with std::remove_cv<T>.

If anyone has any more solutions, tell me and I'll add them here.