Access class members without copying this or mutex variable

Question

I'm trying to create a simple Worker class, which will create a number of threads and a set of tasks for the threads to run. This is educational, but I have already tried asking classmates and searching for different answers, and I think there is something "simple"/fundamental I'm missing here.

I'm having some issues regarding where my mutex variable should be set. I believe I would want one for each of my Worker objects, as the threads created by each object are completely unrelated. Right now, I'm trying to pass this to the lambda method that adds to the set of threads, but this copies the mutex, which deletes it. Is there a way to copy each class member I need independently, or should I create a copy constructor? I'm only trying to pass the class variables I need to the lambda. Here is my code:

// TDAT2004Task2.cpp : Defines the entry point for the application.
//

#include <functional>
#include <iostream>
#include <list>
#include <vector>
#include <mutex>
#include <thread>

using namespace std;

class Worker {
    int threads;

    mutable mutex tasks_mutex;

    vector<thread> worker_threads;
    list<function<void()>> tasks;

public:

    Worker(int threadAmount) {
        threads = threadAmount;
    }


    void start() {
        for (int i = 0; i < threads; i++) {
            //This is the place errors occur. I'm copying the this element, and thus deleting my 
            //mutex variable.
            worker_threads.emplace_back([this] {
                while (true) {
                    function<void()> task;
                    {
                        lock_guard<mutex> lock(tasks_mutex);
                        if (!tasks.empty()) {
                            task = *tasks.begin();
                            tasks.pop_front();
                        }
                    }
                    if (task) {
                        task();
                    }
                }
                });
        }
        for (auto& thread : worker_threads) {
            thread.join();
        }
    }

    void post(function<void()> function) {
        lock_guard<mutex> lock(tasks_mutex);
        tasks.emplace_back(function);
    }
};


int main()
{
    //This does not compile. 
    Worker worker_threads = Worker(4);
}

My error message in VS:

>------ Build All started: Project: TDAT2004Task2, Configuration: x64-Debug ------
  [1/2] C:\PROGRA~2\MICROS~2\2019\COMMUN~1\VC\Tools\MSVC\1424~1.283\bin\HostX64\x64\cl.exe  /nologo /TP   /DWIN32 /D_WINDOWS /W3 /GR /EHsc /MDd /Zi /Ob0 /Od /RTC1 /showIncludes /FoTDAT2004Task2\CMakeFiles\TDAT2004Task2.dir\TDAT2004Task2.cpp.obj /FdTDAT2004Task2\CMakeFiles\TDAT2004Task2.dir\ /FS -c ..\..\..\TDAT2004Task2\TDAT2004Task2.cpp
  FAILED: TDAT2004Task2/CMakeFiles/TDAT2004Task2.dir/TDAT2004Task2.cpp.obj 
  C:\PROGRA~2\MICROS~2\2019\COMMUN~1\VC\Tools\MSVC\1424~1.283\bin\HostX64\x64\cl.exe  /nologo /TP   /DWIN32 /D_WINDOWS /W3 /GR /EHsc /MDd /Zi /Ob0 /Od /RTC1 /showIncludes /FoTDAT2004Task2\CMakeFiles\TDAT2004Task2.dir\TDAT2004Task2.cpp.obj /FdTDAT2004Task2\CMakeFiles\TDAT2004Task2.dir\ /FS -c ..\..\..\TDAT2004Task2\TDAT2004Task2.cpp
C:\Users\Even\source\repos\TDAT2004Task2\TDAT2004Task2\TDAT2004Task2.cpp(74): error C2280: 'Worker::Worker(const Worker &)': attempting to reference a deleted function
  ..\..\..\TDAT2004Task2\TDAT2004Task2.cpp(69): note: compiler has generated 'Worker::Worker' here
  ..\..\..\TDAT2004Task2\TDAT2004Task2.cpp(69): note: 'Worker::Worker(const Worker &)': function was implicitly deleted because a data member invokes a deleted or inaccessible function 'std::mutex::mutex(const std::mutex &)'
  C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.24.28314\include\mutex(92): note: 'std::mutex::mutex(const std::mutex &)': function was explicitly deleted
  ninja: build stopped: subcommand failed.

Error message from cpp in the console:

C:\Users\Even\source\repos\TDAT2004Task2\TDAT2004Task2>gcc TDAT2004Task2.cpp
TDAT2004Task2.cpp: In function 'int main()':
TDAT2004Task2.cpp:74:34: error: use of deleted function 'Worker::Worker(Worker&&)'
  Worker worker_threads = Worker(4);
                                  ^
TDAT2004Task2.cpp:18:7: note: 'Worker::Worker(Worker&&)' is implicitly deleted because the default definition would be ill-formed:
 class Worker {
       ^~~~~~
TDAT2004Task2.cpp:18:7: error: use of deleted function 'std::mutex::mutex(const std::mutex&)'
In file included from /usr/lib/gcc/x86_64-pc-cygwin/7.4.0/include/c++/mutex:43:0,
                 from TDAT2004Task2.cpp:13:
/usr/lib/gcc/x86_64-pc-cygwin/7.4.0/include/c++/bits/std_mutex.h:97:5: note: declared here
     mutex(const mutex&) = delete;
     ^~~~~

Thank you for any responses! Please point at any stupid mistakes I've done, I'm just trying to learn the best practices.

Passing `this` does not copy the object. `this` is a pointer. — user253751, Feb 17 '20 at 12:17
@user253751 I might have thought so. Then why is my mutex variable deleted? When I compile with gcc, I end up with this final error: `mutex(const mutex&) = delete`; — XBullet123, Feb 17 '20 at 12:20
_"error C2280: 'Worker::Worker(const Worker &)': attempting to reference a deleted function"_ and _"error: use of deleted function 'Worker::Worker(Worker&&)' Worker worker_threads = Worker(4);"_ - both explain why you cant' write like this. In short, you need `Worker worker_threads(4);` — mvidelgauz, Feb 17 '20 at 12:27
IMO the diagnostic is wrong here. You provided copy constructor for `Worker`, so you also should provide move constructor (and assignment operators), as per [the Rule of Five](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) — Yksisarvinen, Feb 17 '20 at 12:28
@Yksisarvinen I wouldn't call it _wrong_, it is just too formal. But IMO it isn't compiler primary job to explain about Rule of Five — mvidelgauz, Feb 17 '20 at 12:31
To instantiate a class use `Worker worker_threads(4);` not `Worker worker_threads = Worker(4);`. — frogatto, Feb 17 '20 at 13:00

score 4 · Accepted Answer · answered Feb 17 '20 at 12:38

The problem was, that you defined a copy-constructor. Therefor you do not have a default move contructor.

I do not see how you can have a correct copy-constructor, since std::thread can not be copied, therefore a std::vector<std::thread> can not be copied.

A default-move-constructor should work for your case.

So to be explicit add the lines:

public:

   Worker(const Worker&) = delete;
   Worker& operator=(const Worker&) = delete;
   Worker(Worker&&) = default;
   Worker& operator=(Worker&&) = default;

to your Worker class.

Access class members without copying this or mutex variable

1 Answers1