Builder memory leak when loading

Started by rbenn674, 11 December 2022, 05:31:44

rbenn674

Any ideas on where some of the biggest leakers would be on load? I see a lot of text edit box objects and I feel the toolbox() that gets reloaded may be one of the culprits. I also saw a lot of mouse events in the GUI base and container duplicated. I assume there is a pointer to an object that gets recreated on every load. Just trying to correct anything major that isn't being emptied/deconstructed before a re-loaded state.



Screenshot 2022-12-10 232442.png

texus

I can't immediately think of anything when loading, but there is one thing in the Gui Builder that can probably leak some widgets: callbacks with lambdas.

In TGUI code I try to be careful with callbacks, but for the Gui Builder I think I didn't bother to actually check for potential memory leaks. There are 3 cases that can easily leak:
- Self reference: "button->onPress([=]{ button->...(); });"
- Referencing parent: "button->onPress([=]{ container->... });"
- Circular references: "button->onPress([=]{ editBox->... }); editBox->onTextChange([=]{ button->... });"

If the widgets are accessed by reference instead of by value in the lambda then it wouldn't matter (but then you have to make certain the captured variable doesn't go out of scope before the lambda gets destroyed). With passing the widget by value, the callback stores a copy of the widget. If you have a problematic reference like the 3 cases above then the widget won't be destroyed before its callback is destroyed, but the callback will typically only be destroyed when the widget gets destroyed.

One way to break the circular dependency is to pass a raw pointer to the lambda: "button->onPress([b=button.get()]{ b->...(); });"
But that would need to be looked at on a case-by-case basis, it shouldn't just be done on all lambdas.

It might not be the reason for the particular memory leak you found, but that is one way of easily leaking memory that I'm aware of.

I'll try to look into this memory leak myself as well when I find some time.

rbenn674

#2
I feel at least one item maybe
    for (auto& widget : widgets)
    {
        auto icon = tgui::Picture::create("resources/widget-icons/" + widget.first + ".png");
        auto name = tgui::Label::create(widget.first);
        name->setPosition({icon->getSize().x * 1.1f, "50% - 10"});
        name->setTextSize(14);

        auto verticalLayout = tgui::VerticalLayout::create();
        verticalLayout->setPosition(0, topPosition);
        verticalLayout->setSize({bindWidth(toolbox) - toolbox->getScrollbarWidth(), icon->getSize().y + 4});
        verticalLayout->getRenderer()->setPadding({2});

        auto panel = tgui::Panel::create();
        panel->getRenderer()->setBackgroundColor(tgui::Color::Transparent);
        panel->add(icon);
        panel->add(name);
        verticalLayout->add(panel);
        toolbox->add(verticalLayout);

        panel->onClick([=]{
            createNewWidget(widget 




I need to look into it further but it seems like "widget" is a pointer that creates a bunch of panel objects with lambda calls and then disappears with the objects still existing. Nothing that I can see owns these objects to deconstruct them.

I suppose it gets eventually added to m_gui by copy which should get removed on deconstruction. But I'm not sure about the original created panel and vertical layout, I suppose I need to look at the create() function.

The onClick() does get passed many copies, but they should be deleted on the m_gui->removeAllWidgets().

Screenshot 2022-12-12 215907.png

rbenn674

#3
Found 3 of the bigger ones with leak type you mentioned..

    valueEditBox->onReturnOrUnfocus([=]{ onChange(valueEditBox->getText()); });
    buttonMore->onFocus([=]{ m_propertiesContainer->focusNextWidget(); });
square->onMousePress([=](tgui::Vector2f pos){ onSelectionSquarePress(square, pos); });

 This solved most of the leaks.

texus

Nice. Thanks for looking into this.