I'm attempting to use a C++ class (with constructors & destructors), that has an std::thread
field, in an std::vector
object. I'm using C++ 17 and Visual Studio 2022 to compile all examples shown below.
Here is a simple minimal example that illustrates my issue:
#include <iostream>
#include <thread>
#include <vector>
class TEST
{
public:
std::thread t;
bool done;
TEST()
{
this->done = false;
this->t = std::thread(&TEST::foo, this);
}
~TEST()
{
this->done = true;
this->t.join();
}
void foo()
{
std::cout << "foo\n";
while (!this->done) {};
}
};
int main()
{
std::vector<TEST> vec = {};
vec.push_back(TEST());
vec.clear();
return 0;
}
Where I create a vector vec
in the main
function that contains objects of TEST
class, add one TEST
object, and then immediately clear the vector. The TEST
class contains an std::thread
field called t
, a bool
field called done
and a default constructor and destructor. Each created thread in field t
just calls the foo
function, prints foo
to the console, and waits until the done
field is set to true
by the destructor, and exits. The destructor simply joins the thread.
The specific error I'm prompted with upon a compilation attempt of the above example is as follows:
Error C2280 'TEST::TEST(const TEST &)': attempting to reference a deleted function
It is my understanding that the core reason for the appearance of this error is the fact that the std::thread
class is intentionally non-copyable, and therefore it's copy constructor was explicitly deleted in the standard library. Hence, when the compiler attempts to find the default copy constructor of my TEST
class, it is unable to find std::thread
s copy constructor, and the compilation fails.
However, what I'm struggling to understand is the fact that this error only appears to occur when the default destructor ~TEST
is added by me. Removing it allows the code to compile, albeit the program crashes when ran, I'm assuming because the t
thread field doesn't exit properly upon the object's destruction. I don’t understand the reason as to why this error only manifests when the destructor is explicitly added by me, and doesn’t appear with the implicit empty default constructor.
Another thing I noticed is the fact that this code appears to compile & run without issues without the std::vector
being used, for instance the following example:
#include <iostream>
#include <thread>
class TEST
{
public:
std::thread t;
bool done;
TEST()
{
this->done = false;
this->t = std::thread(&TEST::foo, this);
}
~TEST()
{
this->done = true;
this->t.join();
}
void foo()
{
std::cout << "foo\n";
while (!this->done) {};
}
};
int main()
{
{
TEST test = TEST();
}
return 0;
}
Where I just create a single standalone TEST
object in the main
function, without an std::vector
, and immediately destroy it compiles & executes without issues. It appears that the presence of TEST
objects in an std::vector
simply causes the ~TEST
destructor to fail to compile.
I've also tried using the emplace_back
function of std::vector
instead of push_back
, as suggested by another posting, with the same error appearing.
What is going on here? Why is the presence of objects with std::thread
fields in an std::vector
causing compilation failures, when such standalone objects do not?
As per cppreference, for the move constructor to be implicitly generated for you, there must be
(...) no user-declared destructor.
which is not the case in your example. However,
user may still force the generation of the implicitly declared move constructor with the keyword default.
So, technically you could do:
class TEST {
public:
bool done;
std::thread t;
TEST(const TEST&) = delete;
TEST(TEST&&) = default;
/// rest remains unchanged
Now you will see that there was a very good reason for the compiler to assume that it should not generate the move constructor for you. Trying to
std::vector<TEST> vec = {};
vec.push_back(TEST());
vec.clear();
will cause std::system_error
exception, because in your moved-from object you are still calling t.join()
on the thread that is not joinable anymore. You could try to fix it by:
this->done = true;
if (t.joinable()) {
this->t.join();
}
Even if you take care of this, you are still left with undefined behavior, because of your infinite loop without any side effects:
while (!this->done) {};
You could add a side effect by changing your bool
to std::atomic_bool
, but atomics are not move-constructible so you would need to write your own TEST
move constructor in any case, e.g. using done( other.done.load( ) )
.
EDIT (thanks @Red.Wave)
Even if you solve the issue with moving the atomics, you still have Undefined Behavior, because your moved thread holds captured this
pointer to the moved-from object that no longer exists. Using this pointer, e.g. my calling a member function is UB.
To conclude, better just let your TEST
class to be neither copy, nor move-constructible. You could use a container of smart pointers instead, as mentioned in the answer by @Serge Ballesta.