Memory leak caused by a wrong usage of scoped_lock?

Question

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!

misuse of a locking primitive will more likely cause deadlocks, not memory leaks — Evan Teran, Oct 08 '09 at 02:05
You could just use a simple `std::string` instead of a pointer to it. I don't think the pointer serves any purpose, but I also don't think it's the reason for your memory leak... — sth, Oct 08 '09 at 02:14
@sth: I agree with your comment. However, in cases where one might want to use the string to be mutable, and have changes visible to all references that see it, then a `shared_ptr` is still useful. :-) — C. K. Young, Oct 08 '09 at 03:12

score 0 · Accepted Answer · answered Oct 08 '09 at 02:07

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

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, Oct 08 '09 at 07:34

Memory leak caused by a wrong usage of scoped_lock?

1 Answers1