How can I fix this memory leak when using Qt smart pointers and deleteLater()?

Question

I have a class WorkerThreadManager that creates and owns a number of Qt threads:

class WorkerThread : public QThread
{
    Q_OBJECT
public:
    WorkerThread() : QThread()
    {
        start();
    }

    virtual ~SearchCacheWorkerThread()
    {
        quit();
        wait();
    }
    
    virtual void run() Q_DECL_OVERRIDE
    {
        // fetch from mysql
        // process results
        // append many new items to results_
    
        quit();
    }

private:
    QList<int> results_;
};
typedef QSharedPointer<WorkerThread> WorkerThreadPtr;

class WorkerThreadManager : QObject
{
    Q_OBJECT
public:
    WorkerThreadManager(const int numThreads) : QObject()
    {
        for ( int i = 0; i < numThreads; ++i )
        {
            threads_ << WorkerThreadPtr( new WorkerThread(), &QObject::deleteLater );
        }
    }
    virtual ~WorkerThreadManager() {}

private:
    QList<WorkerThreadPtr> threads_;

When I destroy the WorkerThreadManager object, I don't see the memory being released.

I found from here that the deleteLater won't do anything if the thread has exited the event loop. So I changed the code to remove all calls to quit() in order to ensure a thread stays in the event loop until deleteLater is called. But I still get the memory leak.

1. What's the correct way to destroy the threads (preferrably with the use of smart pointers)?
2. How do I ensure that the threads will finish before they get destroyed, even if their destructor is called?

Answer 1

As JarMan mentioned in a comment, it doesn't seem necessary (at least from the provided snippet) to have a custom deleter for your shared pointer.

If you've already tried removing the deleter, and you still observed the problem, it's possible that someone is still holding a reference to the thread object. To be absolutely sure that's not the case, try using a QScopedPointer instead, which is similar to std::unique_ptr.

This should provide a runtime error if you're passing your smart pointer by value and some other code is attempting to store it, and then you'll know where you should switch to using a raw pointer type (WorkerThread*).

It's hard to know for sure without more code, but a common cause for this issue is capturing a shared pointer by value in a lambda used in a signal/slot connection. Based on what you've provided, is it possible you've done this? I see your workers have the member results_, are you consuming these results on the main thread through a signal emitted from your worker?

Lastly, as Marek R mentioned in the comments, it would be a much safer idea to use worker objects (described in the Qt blog thread they linked) instead of subclassing QThread. QtConcurrent is also an excellent choice if you don't need much control over the thread and only want some calculations to occur in another thread.