c++boostboost-threadscoped-lock

Memory leak caused by a wrong usage of scoped_lock?


I have a memory leak, and I guess it's caused by a wrong usage of scoped_lock (Boost). I however don't manage to find the exact problem, and I do believe that the way the code has been written is not completely right neither.

The code is in this class there: http://taf.codeplex.com/SourceControl/changeset/view/31767#511225

The main important method is ThreadedLoop(). Basically, this method is started within a thread, and checks regularly for market data to be downloaded for Yahoo. For each stock (or else), a new thread will be created (for the ExecuteNextRequest() method), passing as a parameter a pointer to string containing the stock name. This is the only memory allocation I do, but it's released at the end of the thread execution.

I would be also interested in how this code could be enhanced (of course I could use a threadpool, but that's not the point yet). Many thanks!


Solution

  • I suggest that, instead of using a "raw" pointer to std::string, you use a boost::shared_ptr<std::string>, and pass that around. When you're done, call its reset() function; it will decrement the usage count, and free the string automatically when the count is 0.

    As a bonus, you can attach boost::weak_ptr objects to those strings (you can stick them into a vector maybe), and monitor how many of them are still "live". This way you'll know if something's causing the strings to not be decremented to 0 for any reason.

    To be clear:

    if (_tickersQueue.size() > 0)
    {
        boost::shared_ptr<std::string> ticker(new std::string(PopNextTicker()));
        if (!ticker->empty())
            _threads.create_thread(boost::bind(&TAFYahooFinanceParadigm::ExecuteNextRequest, this, ticker));
        else
            ticker.reset();  // optional; ticker will drop out of scope anyway
    }
    

    Yes, you have to adjust the function type of ExecuteNextRequest appropriately. :-)