views:

100

answers:

1

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!

A: 

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. :-)

Chris Jester-Young
You right, using a raw pointer here was useless. However, the memory leak was related to the intensive creations of threads... I implemented a minimalist thread pool and it works a lot better.
TigrouMeow