Qt signal lambda causes shared_ptr leak?

Question

I have the following code:

#include <QApplication>
#include <memory>
#include <QUndoCommand>
#include <QWidget>

class Document
{
public:
    Document()
    {
        qDebug("Document");
    }

    ~Document()
    {
        qDebug("~Document");
    }

    QUndoStack mUndostack;
};

class DocumentRepository
{
public:
    DocumentRepository()
    {
        qDebug("DocumentRepository");
    }

    ~DocumentRepository()
    {
        qDebug("~DocumentRepository");
    }


    void AddDoc(std::shared_ptr<Document> doc)
    {
        mDocs.emplace_back(doc);
    }

    std::vector<std::shared_ptr<Document>> mDocs;
};

class Gui : public QWidget
{
public:
    Gui(DocumentRepository& repo)
     : mRepo(repo)
    {
        qDebug("+Gui");

        for(int i=0; i<3; i++)
        {
            CreateDoc();
        }

        mRepo.mDocs.clear();

        qDebug("-Gui");
    }

    ~Gui()
    {
        qDebug("~Gui");
    }

    void CreateDoc()
    {
        auto docPtr = std::make_shared<Document>();
        connect(&docPtr->mUndostack, &QUndoStack::cleanChanged, this, [=](bool clean)
        {
            // Using docPtr here causes a memory leak on the shared_ptr's, the destruct after ~Gui
            // but without using docPtr here they destruct before ~Gui as exepected.
            QString msg = "cleanChanged doc undo count " + QString::number(docPtr->mUndostack.count());
            qDebug(msg.toLatin1());
        }, Qt::QueuedConnection);
        mRepo.AddDoc(docPtr);
    }

    DocumentRepository& mRepo;
};

int main(int argc, char *argv[])
{
    QApplication a(argc, argv);

    DocumentRepository repo;

    Gui g(repo);
    g.show();

    return 0;
}

Which outputs:

DocumentRepository
+Gui
Document
Document
Document
-Gui
~Gui
~Document
~Document
~Document
~DocumentRepository

But here you can see that the Document instances are leaked as they are destructing after the Gui instance. If you take a look at the comments you'll see I narrowed this issue down to the signal's lambda using the shared_ptr. I want to know why does this cause a leak and how can it be resolved?

For reference the "correct"/non leaking output when not using the shared_ptr in the lambda is:

DocumentRepository
+Gui
Document
Document
Document
~Document
~Document
~Document
-Gui
~Gui
~DocumentRepository

Answer 1

This is an interesting question, let us demystify it:

From the official connect documentation:

The connection will automatically disconnect if the sender is destroyed. However, you should take care that any objects used within the functor are still alive when the signal is emitted.

In your example, you are copying the shared pointer created when used inside the lambda, otherwise there is no copy made for the shared pointer. The copy will naturally increase the reference counter for the object inside the shared pointers. Here is the corresponding documentation from shared_ptr:

The ownership of an object can only be shared with another shared_ptr by copy constructing or copy assigning its value to another shared_ptr

Now, let us differentiate those two cases:

• When you do not copy the shared pointer, there is just one reference to the object and so when the clearing is done for your document repository, there is no more reference to it, and hence the object can be destructed, given that you do not do anything useful inside the lambda function and hence can be optimized out.

• When you copy the shared pointer, there is one reference to object outside the lambad, and there will be one inside, too, due to the shared pointer copy. Now that Qt connection semantics make sure that the object is being kept alive as per the above documentation.

Therefore, when your Gui object gets destructed, it will make all the disconnection as well, and during that period, it can make sure that there is no more reference to the object and hence the destructor called after your gui destructor print statement.

You could probably improve the test code with adding one additional print statement in here:

qDebug("+Gui");
for(int i=0; i<3; i++) CreateDoc();
qDebug("+/-Gui");
mRepo.mDocs.clear();
qDebug("-Gui");

This will also explicitly indicate that to you that the document objects are destructed after repository clear and not at the method termination when you create them. The output will make it more clear:

main.pro

TEMPLATE = app
TARGET = main
QT += widgets
CONFIG += c++11
SOURCES += main.cpp

main.cpp

#include <QApplication>
#include <memory>
#include <QUndoCommand>
#include <QWidget>
#include <QDebug>

struct Document
{
    Document() { qDebug("Document"); }
    ~Document() { qDebug("~Document"); }
    QUndoStack mUndostack;
};

struct DocumentRepository
{
    DocumentRepository() { qDebug("DocumentRepository"); }
    ~DocumentRepository() { qDebug("~DocumentRepository"); }
    void AddDoc(std::shared_ptr<Document> doc) { mDocs.emplace_back(doc); }
    std::vector<std::shared_ptr<Document>> mDocs;
};

struct Gui : public QWidget
{
    Gui(DocumentRepository& repo)
     : mRepo(repo)
    {
        qDebug("+Gui");
        for(int i=0; i<3; i++) CreateDoc();
        qDebug("+/-Gui");
        mRepo.mDocs.clear();
        qDebug("-Gui");
    }

    ~Gui() { qDebug("~Gui"); }

    void CreateDoc()
    {
        auto docPtr = std::make_shared<Document>();
        connect(&docPtr->mUndostack, &QUndoStack::cleanChanged, this, [=](bool) { /* qDebug() << docPtr->mUndostack.count(); */ }, Qt::QueuedConnection);
        mRepo.AddDoc(docPtr);
    }

    DocumentRepository& mRepo;
};

int main(int argc, char *argv[])
{
    QApplication a(argc, argv);
    DocumentRepository repo;
    Gui g(repo);
    g.show();

    return 0;
}

Output

DocumentRepository
+Gui
Document
Document
Document
+/-Gui
~Document
~Document
~Document
-Gui
~Gui
~DocumentRepository

Answer 2

Lambda functions, even though they feel magic, are basically normal functors where captured variables are stored in member variables. Those are copied when you instantiate the lambda function and destroyed with its destruction.

There is a std::shared_ptr<Document> stored in the lambda, copied through [=] since you refer to it in the lambda's body, and the complete lambda itself is copied into the Qt connection along with this shared_ptr.

So it's technically not a leak, you are just holding a reference through an extra shared_ptr instance until the lambda is destroyed, which happens when the first connection emitter or receiver is destroyed (a Gui object in your case).

Since the connection is bound to your Document object, making sure that the lambda only captures a normal pointer or a reference would avoid keeping the Document alive.