Search code examples
c++multithreadingc++11pointersmove

Does move constructor makes any sense for a class that has std::thread's and std::mutex's as its fields?


Suppose I have a class:

class WonnaBeMovedClass
{
public:

    WonnaBeMovedClass(WonnaBeMovedClass&& other);

    void start();

private:

    void updateSharedData();

    std::vector<int> _sharedData;
    std::thread _threadUpdate;
    std::mutex _mutexShaderData;

    //other stuff
};

WonnaBeMovedClass::WonnaBeMovedClass(WonnaBeMovedClass&& other)
{
    _sharedData = std::move(other._sharedData);
    _threadUpdate = std::move(other._threadUpdate);
    _mutexShaderData = std::move(other._mutexShaderData); //won't compile.
}

void WonnaBeMovedClass::start()
{
    _threadUpdate = std::thread(&updateSharedData, this);
}

void WonnaBeMovedClass::updateSharedData()
{
    std::lock_guard<std::mutex> lockSharedData(_mutexShaderData);

    for (auto& value : _sharedData)
    {
        ++value;
    }
}

It won't compile because mutex cannot be moved. It doesn't make sense. Then I thought that it is possible to workaround this by using pointers instead of actual variables and came up with the following:

class WonnaBeMovedClass
{
public:

    WonnaBeMovedClass(WonnaBeMovedClass&& other);

    void start();

private:

    void updateSharedData();

    std::vector<int> _sharedData;
    std::unique_ptr<std::thread> _threadUpdate //pointer;
    std::unique_ptr<std::mutex> _mutexShaderData //pointer;

    //other stuff
};

WonnaBeMovedClass::WonnaBeMovedClass(WonnaBeMovedClass&& other)
{
    _sharedData = std::move(other._sharedData);
    _threadUpdate = std::move(other._threadUpdate);
    _mutexShaderData = std::move(other._mutexShaderData); //won't compile.
}

void WonnaBeMovedClass::start()
{
    _threadUpdate = std::make_unique<std::thread>(&updateSharedData, this);
}

void WonnaBeMovedClass::updateSharedData()
{
    std::lock_guard<std::mutex> lockSharedData(*_mutexShaderData);

    for (auto& value : _sharedData)
    {
        ++value;
    }
}

So now when I:

WonnaBeMovedClass object1;
WonnaBeMovedClass object2;

//do stuff

object1 = std::move(object2);

I actually move addresses of both mutex and thread. It makes more sense now... Or not?

The thread is still working with the data of object1, not object2, so it still doesn't make any sense. I may have moved the mutex, but the thread is unaware of object2. Or is it? I am unable to find the answer so I am asking you for help.

Am I doing something completely wrong and copying/moving threads and mutexes is just a bad design and I should rethink the architecture of the program?

Edit:

There was a question about actual purpose of the class. It is actually a TCP/IP client (represented as a class) that holds:

  • latest data from the server (several data tables, similar to std::vector).
  • contains methods that manage threads (update state, send/receive messages).

More that one connection could be established at a time, so somewhere in a code there is a std::vector<Client> field that represents all active connections. Connections are determined by the configuration file.

//read configurations

...

//init clients
for (auto& configuration : _configurations)
{
    Client client(configuration);
    _activeClients.push_back(client); // this is where compiler reminded me that I am unable to move my object (aka WonnaBeMovedClass object).
}}

I've changed _activeClients from std::vector<Client> to std::vector<std::unique_ptr<Client>> and modified initialization code to create pointer objects instead of objects directly and worked around my issues, but the question remained so I decided to post it here.


Solution

  • Let's break the issue in two.

    1. Moving mutexes. This cannot be done because mutexes are normally implemented in terms of OS objects which must have fixed addresses. In other words, the OS (or the runtime library, which is the same as the OS for our purposes) keeps an address of your mutex. This can be worked around by storing (smart) pointers to mutexes in your code, and moving those instead. The mutex itself doesn't move. Thread objects can be moved so there's no issue.
    2. Moving your own data whereas some active code (a thread or a running function or a std::function stored somewhere or whatever) has the address of your data and can access it. This is actually very similar to the previous case, only instead of the OS it's your own code that holds on the data. The solution, as before, is in not moving your data. Store and move a (smart) pointer to the data instead.

    To summarise,

    class WonnaBeMovedClass
    {
    public:
        WonnaBeMovedClass
            (WonnaBeMovedClass&& other);
        void start();
    private:
        struct tdata {
            std::vector<int> _sharedData;
            std::thread _threadUpdate;
            std::mutex _mutexShaderData;
         };
         std::shared_ptr<tdata> data;
         static void updateSharedData(std::shared_ptr<tdata>);
    };
    
    void WonnaBeMovedClass::start()
    {
        _threadUpdate = std::thread(&updateSharedData, data);
    }