Reputation: 3974
I am searching for a way to start multiple threads whose exact number can only be determined at runtime. The threads are not dependent on each other, so it's a fire-and-forget kind of problem.
The threads do need some context which is stored as internal variables of a class (Foo). Some of these variables are references. The class also holds a method that should be executed as the thread function (bar).
#include <iostream>
#include <string>
#include <vector>
#include <thread>
class Foo
{
public:
Foo(int a){
std::cout << "Created" << std::endl;
m_a = new int(a);
}
~Foo(){
std::cout << "Destroyed" << std::endl;
delete m_a;
}
void bar() {
std::cout << "Internal var: " << *m_a << std::endl;
}
private:
int* m_a;
};
int main() {
for(int i = 0; i < 5; i++) {
std::thread t(&Foo::bar, std::ref(Foo(i)));
// the threads will be joined at a later point, this is for demo purposes
}
return 0;
}
I get a compile error at this point:
error: use of deleted function ‘void std::ref(const _Tp&&) [with _Tp = Foo]’
I get it that this error is caused because of the temporary nature of the object created in the for-loop. But if I remove the std::ref function, I get a segfault: double free or corruption (fasttop)
I am sure that there must be a way of doing this, but I am unaware of that. I would expect some output like (probably in this order, but not guaranteed):
Created
Internal var: 0
Destroyed
Created
Internal var: 1
Destroyed
...
Thanks!
Upvotes: 1
Views: 906
Reputation: 148900
Here is a minimaly fixed version of your code:
Code:
#include <string>
#include <vector>
#include <thread>
#include <iostream>
class Foo
{
public:
Foo(int a) {
std::cout << "Created" << std::endl;
m_a = new int(a);
}
~Foo() {
if (m_a != NULL) {
std::cout << "Destroyed" << std::endl;
delete m_a;
}
}
Foo(const Foo& other) = delete; //not used here
Foo(Foo&& other) {
std::cout << "Move ctor" << '\n';
m_a = other.m_a;
other.m_a = nullptr;
}
void bar() {
std::cout << "Internal var: " << *m_a << std::endl;
}
private:
int* m_a;
};
int main() {
std::vector<std::thread> vec;
for (int i = 0; i < 5; i++) {
std::thread t(&Foo::bar, Foo(i));
vec.push_back(std::move(t));
}
for (auto& t : vec) {
t.join();
}
return 0;
}
Upvotes: 1
Reputation: 85341
Problem 1: Foo
is missing a copy/move constructor. See The rule of three/five/zero.
Add a copy constructor:
Foo(Foo const& that) : m_a(new int(*that.m_a)) {}
And/or a move constructor:
Foo(Foo && that) : m_a(that.m_a) { that.m_a = nullptr; }
Problem 2: Foo(i)
is a temporary instance of Foo
, it lives until the end of the full-expression (the ;
).
std::thread t(&Foo::bar, std::ref(Foo(i)));
// ^
// Foo(i) is dead at this point while the thread is starting!
You want it to live longer than that, in order to be usable inside the thread.
For example, like this (also answers your question about creating threads in a loop):
int main() {
std::vector<Foo> inputs;
std::vector<std::thread> threads;
for(int i = 0; i < 5; i++) {
inputs.emplace_back(i);
threads.emplace_back(&Foo::bar, &inputs.back());
}
for (auto& t : threads) {
t.join();
}
}
Note: std::ref(Foo(i))
doesn't compile because it has protection against returning references to temporaries (precisely to prevent issues like these).
Upvotes: 1
Reputation: 179799
The chief design failure seems that t
is a variable inside the loop. That means it's destroyed at the end of each iteration - you never have 5 std::thread
instances at the same time. Also, you fail to call join
on those threads.
The std::ref
apparently hides this problem and replaces it with another problem, but your original thread creation was correct: std::thread t(&Foo::bar, Foo(i))
.
You probably want a std::list<std::thread>
, and use std::list::emplace_back
to create a variable amount. std::list<std::thread>
allows you to remove threads in any order from the list.
Upvotes: 0