CODEWITHSUNDEEP
c++multithreadingvisual-studioc++11visual-c++
Snens
Snens

Reputation: 33

Possible memory leak by continuously starting threads?

(First, sorry for my bad english.) I've programmed a little test code for my current project and I believe, that I've coded inefficient code: I want to start a new thread (which should run asynchronously to the main thread -> I use detach()), after the user have input three keys on it's keyboard. E.g. this thread should work five seconds and while the thread is working, the user can input more and more keys and always after three inputs a thread should start:

#include <iostream>
#include <cstdlib>
#include <thread>
#include <mutex>

using namespace std;

mutex m;
const int MAX_letters = 3;
int letters = MAX_letters;
thread st;

void send() {
    cout << "\nsending ... [START] ... " << this_thread::get_id() << "\n";
    this_thread::sleep_for(chrono::seconds(5));
    cout << "\nsending ... [END] ...   " << this_thread::get_id() << "\n";
}

int main() {

    while (true) {

        if (letters > 0) {
            system("PAUSE");
            --letters;
        } else {
            st = thread(send);
            st.detach();
            letters = MAX_letters;
        }

    }

    return 0;

}

The system("PAUSE") should simulate the key press...

Now my question: Is it ok to assign a new thread to st even though another is running?

Upvotes: 3

Views: 1198

Answers (3)

SergeyA
SergeyA

Reputation: 62553

Yes, what you are doing is allowed. Since the thread is detached (the use of detached thread is questionable, but I am not discussing this, since it wasn't the question) it's ok to re-assign the thread object. After that, the object will refer to the new thread (until reassigned again). You can check the reference for more information: http://en.cppreference.com/w/cpp/thread/thread/operator%3D

EDIT

Actually you do not even need the variable. std::thread(send).detach() will do.

Upvotes: 2

Gem Taylor
Gem Taylor

Reputation: 5613

The docs don't explicitly say that the thread object is reset to its uninitialised state after detach().

However the docs do say the thread object can be safely destroyed after detach(), and you are using operator= (thread&& rhs) to assign it, which again is documented to destroy the old thread if you hadn't detach()ed it, so all should be good.

But wouldn't it be simpler and clearer to use a local temporary?

   } else {
        thread st(send);
        st.detach();
        letters = MAX_letters;
   }

Upvotes: 0

Soleil
Soleil

Reputation: 7289

There is no problem with starting threads like this, however, you might want to add a condition where you exit the while(true) loop. detach() allows the threads resources to be recycled, so there is no memory leak here.

Upvotes: 0

Related Questions