CODEWITHSUNDEEP
c++lambdashared-ptr
Barış Uşaklı
Barış Uşaklı

Reputation: 13532

Passing shared_ptr to lambda by value leaks memory

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.

Upvotes: 17

Views: 18138

Answers (2)

Victor Laskin
Victor Laskin

Reputation: 2647

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

Upvotes: 1

Cort Ammon
Cort Ammon

Reputation: 10873

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

Upvotes: 21

Related Questions