c++ program potential memory leak

Question

I call the following void function() function every 40ms and I found that the memory consumption increase steadily. The consumption is not obvious at first but after days, the consumption is huge. Can anyone help to explain what is wrong with this code. Is it a thread problem or std::move problem that caused the memory leak.

void do_task(const std::vector<int>& tmp)
{
    // do some work here
}

void function()
{
    std::vector<std::thread> task;
    std::vector<int> tmp1, tmp2;

    GetTempValue(tmp1);
    GetTempValue(tmp2);

    task.push_back(std::thread(do_task, std::move(tmp1)));
    task.push_back(std::thread(do_task, std::move(tmp2)));

    tmp1.clear();
    tmp2.clear();

    UpdateTempValue(tmp1);
    UpdateTempValue(tmp2);

    task.push_back(std::thread(do_task, std::move(tmp1)));
    task.push_back(std::thread(do_task, std::move(tmp2)));

    tmp1.clear();
    tmp2.clear();

    for(int i=0; i<task.size(); i++)
    {
        task[i].join();
    }
}

I don't suppose you sent this through Valgrind? Regardless, if move-semantics are doing their job, those clears are pointless. Out of curiosity please update your question with your platform toolchain info, and ideally an mcve. — WhozCraig
@rxu. There cannot be a data race issue with code above. The vectors tmp1 and tmp2 are handled quite correctly. Except for the fact that the calls to clear() are redundant. — Michaël Roy
I would take a harder look into do_task(). What are the OS and compiler used here? — Michaël Roy
Wait a minute!!! Shouldn't do_task receive a std::vector, and not a reference? — Michaël Roy

Michaël Roy Michaël Roy · Accepted Answer · 2017-08-13T04:18:34

Passing a reference to a thread is a big no-no. Or at least a bug waiting to happen...

You should try redefining do task to accept a std::vector by value. Isn't that what you were trying to do anyway by calling std::move to pass them to the threads?

Change the definition of do_task to:

void do_task(std::vector<int> tmp);

I've made some quick calculations. If the threads started by function() leak, by starting 4 threads every 40ms, you should have a leak rate of over 1.4MB/hour. If your leak is less than that, you should start looking elsewhere in your code.

In any case, starting 100 threads per second is not really efficient. A big chunk of your computing power is lost in creating threads. Have you considered other options?

Having 4 threads running endless loops, and queuing the work to them will be way more efficient, less taxing on the OS, and less prone to leaks that you cannot control.

c++ program potential memory leak

2 Answers