c++multithreadingc++11gcc4.8

Move operations for a class with a thread as member variable


I'm trying to expand with what some people helped me here Call function inside a lambda passed to a thread so my worker class can support a move constructor and a move operator=, but I have the problem that my class is binding this by copy (or reference) to the thread so it can access the class values. Which are several atomic<bool>, a condition_variable and a mutex.

But when I try to move it since the thread is bound to the other condition variable, mutex and atomics, anything I do on it is not working. How can I fix this? Do I need to use a more complex object and move it around instead of a lambda so the thread can have a referenece to it? or is there another alternative. As always help will be really appreciated :).

Here is a snippet (MWE) of the implementation.

class worker {
public:
    template <class Fn, class... Args>
    explicit worker(Fn func, Args... args) {
        t = std::thread(
            [&func, this](Args... cargs) -> void {
                std::unique_lock<std::mutex> lock(mtx);
                while (true) {
                    cond.wait(lock, [&]() -> bool { return ready; });

                    if (terminate)
                        break;

                    func(cargs...);

                    ready = false;
                }
            },
            std::move(args)...);
    }

    worker(worker &&w) : t(std::move(w.t)) { /* here there is trouble  */ }

    worker &operator=(worker &&w) {
        t = std::move(w.t);
        terminate.store(wt.terminate);
        ready.store(wt.ready);
        return *this; 
        /* here too */
    }

    ~worker() {
        terminate = true;
        if (t.joinable()) {
            run_once();
            t.join();
        }
    }

    worker() {}

    void run_once() {
        std::unique_lock<std::mutex> lock(mtx);
        ready = true;
        cond.notify_one();
    }

bool done() { return !ready; }

private:
    std::thread t;
    std::atomic<bool> ready, terminate; // What can I do with all these?
    std::mutex mtx;                     //
    std::condition_variable cond;       //
};

int main() {
    worker t;
    t = worker([]() -> void { cout << "Woof" << endl; });
    t.run_once();
    while(!t.done()) ;
    return 0;
}

Sorry for the big dump of code.


Solution

  • I would 'fix' it by just saying worker is noncopyable and nonmoveable and leave it to the user of worker to store it as a unique_ptr if they want to move it around. There's nothing wrong with that at all. It's ordinary, actually.

    If you absolutely want this class to be movable, you could use the Pimpl design pattern: make a Worker::Impl nested class which worker owns by unique_ptr. The Impl class would be noncopyable and nonmovable and basically be your current worker class. The lambda would have a pointer to the Impl class, not the worker class. The worker class would contain nothing except a unique_ptr to the Impl and functions that forward to the Impl class' functions, and the defaulted copy and move ctors/operators will just work properly for both classes (worker will be copyable but not movable, impl will be noncopyable and nonmovable).