Remove thread from vector upon thread completion

Question

I have a vector of threads in my C++ program.

std::vector<std::thread> threadList;

I then create a thread and push it into the vector.

threadList.push_back(std::thread([]() { ... }));

How can I remove the thread from the threadList vector when the lambda function finishes execution?

Edit

I've come up with somewhat of a solution; Before the thread lambda function returns, it iterates through the vector to find a match in ID with this_thread::get_id().

Through Visual Studio debugging line by line, I see that it finds a match in ID and the erase function is executed, but as soon as threadList.erase(threadList.begin() + index); is executed, I come accross an unhandled exception at the thread's deconstructor function.

I've written a small piece of code which replicates this error.

vector<thread> threadList;

threadList.push_back(thread([]() {
    Sleep(1000);
    threadList.erase(threadList.begin());
}));

Sleep(2000);

//for_each(threadList.begin(), threadList.end(), mem_fn(&thread::detach));
//threadList.clear();

This code results in the screenshot below.

The lambda can remove the `std::thread` from the vector (which means protecting the `vector` from concurrent access, such as with a `std::mutex`) before exiting. Or, you can have the lambda signal the thread that owns the `vector` in some way, and let that thread remove the `std::thread` when it has time to do so. Or, you can have the owning thread (or another monitoring thread) simply `join()` the `std::thread` and remove it when it finishes. — Remy Lebeau, Sep 13 '17 at 01:34
`join()` the thread first, then remove it from the vector in exactly, 100% identical fashion you would remove something from any other vector. A vector does not behave any differently, in terms of adding or removing values from it, just because it contains `std::thread`s. — Sam Varshavchik, Sep 13 '17 at 01:35
What you're doing with `push_back` (in the 'edit' part) is invalid; modifying an object in multiple threads. The first comment already mentioned that — LWimsey, Sep 13 '17 at 03:38
@RemyLebeau when you say I can signal the thread that owns the vector... do you have any idea how I could do that? Possibly a variable being checked in an infinite loop with a delay? — Ari Seyhun, Sep 13 '17 at 03:57
@Acidic: let me backstep a little. The lambda cant remove the `std::thread` *directly*, because the thread is still running and the `std::thread` destructor calls `std::terminate()` if the `std::thread` is joinable. So you have to do the removal asynchronously. There are plenty of different ways to handle this. `std::async`, for example. And don't forget to wrap the `vector` with a `std::mutex` or similar thread-safe lock so multiple threads can't modify the `vector` at the same time. — Remy Lebeau, Sep 13 '17 at 04:06
@RemyLebeau I'm having a lot of trouble with this :/ I can't figure out how to do what you're saying — Ari Seyhun, Sep 13 '17 at 04:08
Remmy boi.......................................... you just solved it — Ari Seyhun, Sep 13 '17 at 04:28

Remy Lebeau · Accepted Answer · 2017-09-13T07:00:37.217

One option is to have the lambda remove the thread asynchronously when exiting. For example:

std::vector<std::thread> threadList;
std::mutex threadMutex;

... 

void removeThread(std::thread::id id)
{
    std::lock_guard<std::mutex> lock(threadMutex);
    auto iter = std::find_if(threadList.begin(), threadList.end(), [=](std::thread &t) { return (t.get_id() == id); });
    if (iter != threadList.end())
    {
        iter->detach();
        threadList.erase(iter);
    }
}

... 

{
    std::lock_guard<std::mutex> lock(threadMutex);
    threadList.push_back(
        std::thread([]() {
            ...
            std::async(removeThread, std::this_thread::get_id());
        })
    );
}

Alternatively:

std::vector<std::thread> threadList;
std::mutex threadMutex;

... 

void removeThread(std::thread::id id)
{
    std::lock_guard<std::mutex> lock(threadMutex);
    auto iter = std::find_if(threadList.begin(), threadList.end(), [=](std::thread &t) { return (t.get_id() == id); });
    if (iter != threadList.end())
    {
        iter->join();
        threadList.erase(iter);
    }
}

... 

{
    std::lock_guard<std::mutex> lock(threadMutex);
    threadList.push_back(
        std::thread([]() {
            ...
            std::thread(removeThread, std::this_thread::get_id()).detach();
        })
    );
}

Alternatively:

std::vector<std::thread> threadList;
std::mutex threadMutex;

std::list<std::thread::id> threadFreeList;
std::mutex threadFreeMutex;
std::condition_variable threadFreeCV;

std::thread monitor([]() {
    while (... /* app is not terminated */)
    {
        std::unique_lock<std::mutex> lock(threadFreeMutex);
        threadFreeCV.wait(lock);

        std::lock_guard<std::mutex> lock2(threadMutex);
        auto iter = threadFreeList.begin();
        while (iter != threadFreeList.end())
        {
            auto id = *iter;
            auto found = std::find_if(threadList.begin(), threadList.end(), [=](std::thread &t) { return (t.get_id() == id); });
            if (found != threadList.end())
            {
                found->join();
                threadList.erase(found);
            }
            iter = threadFreeList.erase(iter);
        }
    } 
});

...

{
    std::lock_guard<std::mutex> lock(threadMutex);
    threadList.push_back(
        std::thread([]() {
            ...
            std::unique_lock<std::mutex> lock(threadFreeMutex);
            threadFreeList.push_back(std::this_thread::get_id());
            threadFreeCV.notify_one();
        })
    );
}

You lock the mutex but never unlock it... is this an error in your code or intentional? — Ari Seyhun, Sep 13 '17 at 04:53
@Acidic: intentional. RAII semantics takes care of unlocking it: "*When control leaves the scope in which the lock_guard object was created, the lock_guard is destructed and the mutex is released.*" STL classes are built on the principles of RAII. — Remy Lebeau, Sep 13 '17 at 05:02
In my project, I have code below the `threadList.push_back` statement but still inside the scope. Should I unlock in that case? — Ari Seyhun, Sep 13 '17 at 05:05
@Acidic: `lock_guard` cannot be unlocked manually, only when it goes out of scope. So wrap the `push_back` inside a new scope (a pair of brackets) and put the `lock_guard` inside of that scope, eg: `{ ... { lock_guard; push_back; } ... }` — Remy Lebeau, Sep 13 '17 at 05:07
This doesn't work because `async` returns a `future` and this future's destructor blocks until `removeThread` has finished --> deadlock — LWimsey, Sep 13 '17 at 06:14
@LWimsey: if `removeThread()` uses `detach()` instead of `join()`, it shouldn't deadlock anymore. Otherwise, use a detached `std::thread` instead of `std::async`. See [Can I use std::async without waiting for the future limitation?](https://stackoverflow.com/questions/21531096/) — Remy Lebeau, Sep 13 '17 at 06:18

score 0 · Answer 2 · answered Sep 13 '17 at 06:34

Why do you need this vector of threads when you can detach them?

// Scope that outlives the threads
boost::barrier out_barrier(N_threads+1);

...

// Starting the threads
for(int i = 0; i < N_threads; i++) {
    std::thread th([&out_barrier]() {
        ...do the job...
        out_barrier.wait();
    });
    th.detach();
}

...

// Wait for threads to finish
out_barrier.wait();

The threads are not joinable so it's safe to call the destructor. The synchronization is unavoidable in this case. In my example, it's used to join all threads and if you have a vector of threads you will need to synchronize the access to the vector so it's all the same.

Remove thread from vector upon thread completion

2 Answers2