Passing shared_ptr to lambda by value leaks memory

Question

I have the following code :

void MyClass::onOpenModalBtnClicked() {
    uiManager->load(L"data/ui/testmodal.json");
    std::shared_ptr<UIElement> modal = uiManager->getElementById("loginModal");

    if(modal) {
        modal->getElementById("closeButton")->onClicked = [modal]() {
            modal->hide();
        };
    }
}

This works fine and the modal is closed when the button is clicked, onClicked is a std::function.

I also have this at the beginning of my app :

#if defined(DEBUG) | defined (_DEBUG)
    _CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
#endif

This prints out memory leaks when the app terminates.

With the above code I get lots of memory leaks, if I change the code to the below they are all gone :

void MyClass::onOpenModalBtnClicked() {
    uiManager->load(L"data/ui/testmodal.json");
    std::shared_ptr<UIElement> modal = uiManager->getElementById("loginModal");

    if(modal) {
        modal->getElementById("closeButton")->onClicked = [this]() {
            uiManager->getElementById("loginModal")->hide();
        };
    }
}

I am assuming passing in the shared_ptr by value increases the ref count by 1 and then this reference never goes out of scope or it goes out of scope after the mem leaks are reported. So I tried to call reset inside the lambda after I used the shared_ptr but then I get this compiler error :

Error 1 error C2662: 'void std::shared_ptr<_Ty>::reset(void) throw()' : cannot convert 'this' pointer from 'const std::shared_ptr<_Ty>' to 'std::shared_ptr<_Ty> &'

So the question is how can I use the captured modal and not get those memory leaks?

Edit: So I got rid of the compile error by adding mutable to the lambda.

if(modal) {
    modal->getElementById("closeButton")->onClicked = [modal]() mutable {
        modal->hide();
        modal.reset();
    };
}

Now if I click the close button, and close the app there are no memory leaks, since the reset cleans that reference. But if the button is never clicked I still get the leaks.

Are you positive that the object holding onto onClicked is being destroyed properly? There would be no way to destroy the `shared_ptr` until onClicked is destroyed. — Cort Ammon, Sep 15 '13 at 23:02
The destructor for the close button is never called if I capture `modal`, actually none of the destructors are called for anything that is a child of modal. Which makes me believe the captured `modal` never goes out of scope. — Barış Uşaklı, Sep 15 '13 at 23:05
Thank you, that was the piece I wasn't seeing. Answer incoming. — Cort Ammon, Sep 15 '13 at 23:07

score 21 · Accepted Answer · answered Sep 15 '13 at 23:10

You have created a shared_ptr cycle.

modal cannot be destroyed until its reference count hits 0. You then pass a copy of a shared_ptr to modal into the labmda function, incrementing its reference count. You then assign that lambda function into a member of modal.

This means that modal is always referred to by its callback function. However, its callback function cannot be destroyed until modal has no refcount. Modal ends up getting stuck with a ref count of 1.

The usual solution is to pass either a naked pointer or (preferrably) a weak pointer into the lambda

score 1 · Answer 2 · answered Oct 22 '14 at 17:18

No.

As solution for this problem i have the following simple test:

class Modal {
public:
    Modal(){ onClick = nullptr; }
    std::function<void()> onClick;
};

class Data {
public:
    string* message;
    Data() { message = nullptr; }
    Data(string s) { message = new string(s); LOG << "CREATED" << NL; }
    Data(Data&& d) { LOG << "MOVE CTR" << NL; message = d.message; d.message = nullptr;}
    Data(const Data& d) { LOG << "COPY CTR" << NL; message = new string(*d.message); }
    virtual ~Data() { if (message) delete message; LOG << "DESTROYED" << NL; }
};


{
    Modal modal;
    {
        std::shared_ptr<Data> p = std::make_shared<Data>(Data("Will it be deleted?"));
        LOG << *(p->message) << " " << p.use_count() << NL;
        modal.onClick = [p](){
            LOG << *(p->message) << " " << p.use_count() << NL;
        };

        modal.onClick();
    }

    modal.onClick();
    modal.onClick = nullptr;
    LOG << "End of test" << NL;
}

Where i get the following picture as output:

Test output

As you can see when you overwrite onClick handler destroy event is called. So there are no need for any reset() calls inside lambda body. See reference counter output. The lambda is functor object and is properly destroyed when holder object (modal in example) no longer exists or field is cleared (or updated).

Passing shared_ptr to lambda by value leaks memory

2 Answers2

Linked