vector becomes empty when the operator() is executed by std::thread

Question

I’m trying to create a thread (PrinStringManager) which in turn creates several threads (PrintEntry) (depending on the number of elements of the incoming vector of strings). Each PrintEntry thread created simply prints the string received in the constructor.

It is just a small example representing my problem.

class PrintEntry
{
public:
    PrintEntry(std::string& name) :name_(name) {}
    ~PrintEntry() {}
    void operator()() {
        std::cout << "name: " << name_;
    }
private:

    std::string name_;
};

class PrinStringManager
{
public:
    PrinStringManager(std::vector<std::string>& vec) :vec_(vec){}
    ~PrinStringManager() {
        for (auto& t : vt_) t.join();
    }

    void operator()() {
        for (auto name : vec_)
        {
            vt_.emplace_back(PrintEntry{ name });
        }
    }

private:

    std::vector<std::string> vec_;
    std::vector<std::thread> vt_;
};


void f()
{
    std::vector<std::string> vec{ "one","two", "three" };
    PrinStringManager psm{ vec };
    std::thread t{ std::ref(psm) };
    t.detach();
} 

int main()
{
    f();
    while (true)
    {
        std::this_thread::sleep_for(std::chrono::milliseconds(1000));
    }
    std::cout << "Hello World!\n";
}

What it is happening, is that the object PrinStringManager is created with a vector of 3 strings but when the object function (from PrintStringmanager) is invokek by the thread, the vector is empty:

void operator()() {
    for (auto name : vec_) //<-- vec is empty!!
    {
        vt_.emplace_back(PrintEntry{ name });
    }
}

I noticed that when the end of scope for function f() is reached the destructor of PrinStringManager is called and that should be the problem.

Is there a way to o overcome this problem in a simply manner without setting PrinStringManager as static so that it lives forever?

Is there a way to move psm inside the thread object so that when the end of scope of f() is reached, the psm inside the thread hold the original value?

Yes, create the `PrintStringManager` in `main` and pass a reference to it to `f`. — john, Feb 23 '23 at 11:28
"I noticed that when the end of scope for function f() is reached the destructor of `PrinStringManager` is called and that should be the problem." No not really, because the `PrinStringManager` destructor will not return until all the threads created by the manager have been joined. — Some programmer dude, Feb 23 '23 at 11:29
What *is* a problem is that you pass a reference to the local `psm` object to your detached thread. First it's a problem because the threads created by `psm` might have ended before `t` even starts. But it's a bigger problem that the life-time of `pfm` could end before the thread `t` finishes, and leave you with an invalid reference. — Some programmer dude, Feb 23 '23 at 11:29
@Some programmer dude So, should i use static PrinStringManager psm; instead? — eniac, Feb 23 '23 at 11:44
this is the problem with using `thread.detach()` it makes it much easier to mishandle object lifetimes. You're storing a reference to the local variable `vec` which ceases to exist at the end of `f()`, `psm` is also a local variable so storing the reference to that doesn't work either — Alan Birtles, Feb 23 '23 at 11:46
@AlanBirtles But the vector of string is copied inside PrintStringManager! And psm will cease to exist.... — eniac, Feb 23 '23 at 11:50
ah, ok, if its copied then why is it passed by lvalue reference rather than const reference or r-value reference — Alan Birtles, Feb 23 '23 at 11:51
no, passing the vector into psm by l-value is confusing, conventionally it would be a const l-value or r-value. It won't fix anything but it makes your code clearer — Alan Birtles, Feb 23 '23 at 11:56
@AlanBirtles is there a way to move psm inside the thread object so that when the end of scope of f() is reached, the psm inside the thread hold the original value? — eniac, Feb 23 '23 at 12:04

Artyer · Accepted Answer · 2023-02-23T12:17:51.610

First make your object movable by adding a move constructor:

    // No implicitly declared one since you have a destructor, so bring it back
    PrinStringManager(PrinStringManager&&) = default;
    // Also the move assign is probably wanted
    PrinStringManager& operator=(PrinStringManager&&) = default;

// (And also fix `PrintEntry`, probably by removing the destructor)

Then you can make the thread hold its own PrinStringManager object, instead of merely a reference to one on the stack:

// The thread will hold a PrinStringManager move-constructed from psm
std::thread t{ std::move(psm) };

If you can't make PrinStringManager movable for whatever reason, you can use dynamic allocation:

void f()
{
    std::vector<std::string> vec{ "one","two", "three" };
    auto psm = std::make_unique<PrinStringManager>(vec);
    // The thread holds a pointer to `psm` which is deleted
    // when the thread finishes
    std::thread t{ [psm=std::move(psm)]{ (*psm)(); } };
    t.detach();
}

Already tried that but the compiler complains with: `error C2280: 'std::thread::thread(const std::thread &)': attempting to reference a deleted function` — eniac, Feb 23 '23 at 12:09
@eniac Turns out your class wasn't movable (See https://stackoverflow.com/q/33932824 for more information, and follow the rule of 0/3/5) — Artyer, Feb 23 '23 at 12:19

vector becomes empty when the operator() is executed by std::thread

1 Answers1