Memory issues when ImGui::TabLabels are stored in a vector

[questor]

I've triggered by incommon usage a memory use after free by having a nodegrapheditor in an own class as member and moving my object into a std-vector. the nodegrapheditor inherits from TabLabel which has pointer-members to label, tooltip and userText. I think my sequence to trigger the error is: create nodegrapheditor and by that a tablabel with a label-string. now I move this nodegrapheditor into the vector which calls the (implicit) copy-constructor of tablabel which copies the pointers, afterwards the old object gets deleted and calls the MemFree-functions, but the pointers are still used in the copied object and are now unsafe to use.

I've written a copy-constructor and a copy assignment operator to fix it:

    TabLabel(const TabLabel &other) {
        label = tooltip = userText = NULL;
        userPtr = other.userPtr;
        userInt = other.userInt;
        closable = other.closable;
        draggable = other.draggable;
        mustCloseNextFrame = other.mustCloseNextFrame;
        mustSelectNextFrame = other.mustSelectNextFrame;
        wndFlags = other.wndFlags;
        modified = other.modified;
        if(other.label) {
            label = (char*)ImGui::MemAlloc(strlen(other.label)+2);
            strcpy(label, other.label);
        }
        if(other.tooltip) {
            tooltip = (char*)ImGui::MemAlloc(strlen(other.tooltip)+1);
            strcpy(tooltip, other.tooltip);
        }
        if(other.userText) {
            userText = (char*)ImGui::MemAlloc(strlen(other.userText)+1);
            strcpy(userText, other.userText);
        }
    }
    TabLabel &operator=(const TabLabel &other) {
        if(this != &other) {
            label = tooltip = userText = NULL;
            userPtr = other.userPtr;
            userInt = other.userInt;
            closable = other.closable;
            draggable = other.draggable;
            mustCloseNextFrame = other.mustCloseNextFrame;
            mustSelectNextFrame = other.mustSelectNextFrame;
            wndFlags = other.wndFlags;
            modified = other.modified;
            if(other.label) {
                label = (char*)ImGui::MemAlloc(strlen(other.label)+2);
                strcpy(label, other.label);
            }
            if(other.tooltip) {
                tooltip = (char*)ImGui::MemAlloc(strlen(other.tooltip)+1);
                strcpy(tooltip, other.tooltip);
            }
            if(other.userText) {
                userText = (char*)ImGui::MemAlloc(strlen(other.userText)+1);
                strcpy(userText, other.userText);
            }
        }
        return *this;
    }

with that included I have no longer the memory usage after free in my codebase....

[Flix01]

the nodegrapheditor inherits from TabLabel

Wow, you're using both ImGui::TabWindow and ImGui::NodeGraphEditor together in an advanced way (this has never been showed in the demos, as far as I remember)!

The TabLabel-inheritance was something I had in mind since the beginning of the addon branch, but I've never fully developed it.

In any case... thank you very much for your contribution! I'll merge your suggested additions with next commit!

P.S. I remember that the inheritance of ImGui::TabLabel in ImGui::NodeGraphEditor is partial, because implementing a "modified" feature for ImGui::NodeGraphEditor was too difficult (there's no Undo/Redo stack). Be warned!

[Flix01]

@questor Could you please try these modifications and see if there are memory leaks?

TabLabel(const TabLabel& o) {
    label = tooltip = userText = NULL;
    userPtr = NULL;userInt=0;
    closable = draggable = true;
    mustCloseNextFrame = false;
    mustSelectNextFrame = false;
    wndFlags = 0;
    modified = false;
    *this=o;
}

TabLabel& operator=(const TabWindow::TabLabel &o) {
    if(this != &o) {
        userPtr = o.userPtr;
        userInt = o.userInt;
        closable = o.closable;
        draggable = o.draggable;
        mustCloseNextFrame = o.mustCloseNextFrame;
        mustSelectNextFrame = o.mustSelectNextFrame;
        wndFlags = o.wndFlags;
        modified = o.modified;
        setLabel(o.label,o.modified);
        setTooltip(o.tooltip);
        setUserText(o.userText);
    }
    return *this;
}

Another thing to consider is that we should put this code in the public section of the TabLabel struct, that is not very elegant, but if it's necessary we must do it.

[Flix01]

@questor Note that I've just edited the post above and I've slightly modified the constructor: I hope it's OK to leave the strings to NULL.

[Flix01]

I've posted a possible fix. Feel free to reopen this issue if necessary.

[questor]

thanks!