Double freeing of a component

1

I’m pretty sure this is not a problem with JUCE, but am not being able to solve it from three days.If I manually delete a component I am getting a double drop error, if I did not, the exact node and it’s contents are leaking, how is this possible, they do not share any memory with other components or not being deleted multiple times. I ran it through valgrind but still am as clueless. Appreciate any help.

 Invalid free() / delete / delete[] / realloc()
==139810==    at 0x484BB6F: operator delete(void*, unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==139810==    by 0x1EC526: std::default_delete<ParameterCtrl>::operator()(ParameterCtrl*) const (unique_ptr.h:85)
==139810==    by 0x1E9B57: std::unique_ptr<ParameterCtrl, std::default_delete<ParameterCtrl> >::~unique_ptr() (unique_ptr.h:361)
==139810==    by 0x1E6A2D: Socket::~Socket() (Socket.h:64)
==139810==    by 0x1E6A7D: Socket::~Socket() (Socket.h:64)
==139810==    by 0x1EF8F6: juce::ContainerDeletePolicy<Socket>::destroy(Socket*) (juce_ContainerDeletePolicy.h:54)
==139810==    by 0x1ECA2A: juce::OwnedArray<Socket, juce::DummyCriticalSection>::deleteAllObjects() (juce_OwnedArray.h:815)
==139810==    by 0x1ECC0B: juce::OwnedArray<Socket, juce::DummyCriticalSection>::clearQuick(bool) (juce_OwnedArray.h:120)
==139810==    by 0x1EA281: juce::OwnedArray<Socket, juce::DummyCriticalSection>::clear(bool) (juce_OwnedArray.h:109)
==139810==    by 0x1E79AC: GraphNode::~GraphNode() (GraphNode.h:187)
==139810==    by 0x1E89E9: Oscillator::~Oscillator() (Oscillator.h:61)
==139810==    by 0x1E8A09: Oscillator::~Oscillator() (Oscillator.h:61)
==139810==  Address 0x69dea60 is 448 bytes inside a block of size 512 alloc'd
==139810==    at 0x4849013: operator new(unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==139810==    by 0x1E6B95: Socket::addMenuParameterControl() (Socket.h:108)
==139810==    by 0x1E842F: Oscillator::Oscillator(int, int) (Oscillator.h:26)
==139810==    by 0x1E2AC3: Instrument::GraphPage::AddNodeCallback(int, Instrument::GraphPage*) (Instrument.cpp:358)
==139810==    by 0x1EBE2D: juce::ModalCallbackFunction::forComponent<Instrument::GraphPage>(void (*)(int, Instrument::GraphPage*), Instrument::GraphPage*)::{lambda(int)#1}::operator()(int) const (juce_ModalComponentManager.h:281)
==139810==    by 0x1F1822: void juce::NullCheckedInvocation::invoke<juce::ModalCallbackFunction::forComponent<Instrument::GraphPage>(void (*)(int, Instrument::GraphPage*), Instrument::GraphPage*)::{lambda(int)#1}, int&>(juce::ModalCallbackFunction::forComponent<Instrument::GraphPage>(void (*)(int, Instrument::GraphPage*), Instrument::GraphPage*)::{lambda(int)#1}&&, int&) (juce_Functional.h:65)
==139810==    by 0x1EE8D2: juce::ModalCallbackFunction::create<juce::ModalCallbackFunction::forComponent<Instrument::GraphPage>(void (*)(int, Instrument::GraphPage*), Instrument::GraphPage*)::{lambda(int)#1}>(juce::ModalCallbackFunction::forComponent<Instrument::GraphPage>(void (*)(int, Instrument::GraphPage*), Instrument::GraphPage*)::{lambda(int)#1}&&)::Callable::modalStateFinished(int) (juce_ModalComponentManager.h:179)
==139810==    by 0x6BBE70: juce::ModalComponentManager::handleAsyncUpdate() (juce_ModalComponentManager.cpp:210)
==139810==    by 0x5753AE: juce::AsyncUpdater::AsyncUpdaterMessage::messageCallback() (juce_AsyncUpdater.cpp:34)
==139810==    by 0x577EB0: juce::InternalMessageQueue::InternalMessageQueue()::{lambda(int)#1}::operator()(int) const (juce_Messaging_linux.cpp:42)
==139810==    by 0x58B30D: void std::__invoke_impl<void, juce::InternalMessageQueue::InternalMessageQueue()::{lambda(int)#1}&, int>(std::__invoke_other, juce::InternalMessageQueue::InternalMessageQueue()::{lambda(int)#1}&, int&&) (invoke.h:61)
==139810==    by 0x587D6A: std::enable_if<is_invocable_r_v<void, juce::InternalMessageQueue::InternalMessageQueue()::{lambda(int)#1}&, int>, void>::type std::__invoke_r<void, juce::InternalMessageQueue::InternalMessageQueue()::{lambda(int)#1}&, int>(juce::InternalMessageQueue::InternalMessageQueue()::{lambda(int)#1}&, int&&) (invoke.h:111)

The allocation and freeing is for the same component here.

2

I think we need the to see the code for Socket and ParameterCtrl.

Also, what exactly do you mean by “manually delete a component”? Seems like you are using smart pointers, so C++ should do all the memory management for you.
And how do you know it’s a double free issue? That report doesn’t seem to say that, just that it’s invalid which could be for a lot of reasons.

3

The code for the ParameterCtrl is :

class ParameterCtrl {
public:

    ParameterCtrl() {}

    ~ParameterCtrl() {}


    // getValue() is the abstraction that is called from process in the respective node to return the
    // selected item or a value.
    //
    // and because floating points can have precision errors better to compare with a greater than or less than
    // rather than equals to if you mean to return a integer.
    virtual float getValue() {
        return 0.0;
    };

    // basically like a resize
    virtual void setBoundForCtlr(juce::Rectangle<int> bound) {}

    virtual void paintAgain() {}

    // lock and unlock while reading the values.
    void lock() { mutex.lock(); }

    void unlock() { mutex.unlock(); }

    std::unique_ptr<juce::LookAndFeel_V4> styles;

    virtual int getType() { return -1; }

private:
    // This is to lock this when others are reading or setting it.
    std::mutex mutex;

    JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (ParameterCtrl)
};

One of the sub class inherits ComboBox and one inherits TextEditor, the super class does not inherit a Component because it makes functions like repaint ambiguous in sub classes.

And for the Socket :

#include <JuceHeader.h>
#include "InputOutputTypesForSokets.h"
#include "ParameterCtrl.h"

//==============================================================================
/*
*/
class Socket  : public juce::Component {
public:

    direction dir;

    // if this is empty then we do not draw a socket input for this socket.
    std::set<SocketDataType> TypesAccepted;

    juce::String name;

    // isMust must be connected or else the queue will not be built.
    // for example an empty input for audio signal will stop the whole process.
    Socket( juce::String name,
            direction dir,
            bool isMust) {

        this->name = name;
        this->dir = dir;
        this->isMust = isMust;

        parameterController = nullptr;

        setBounds(0, 0, 5, 5);
    }

    // Be careful while using this,
    // The program will have undefined behaviour if there is no implementation for this type in the process method.
    void acceptType(SocketDataType type) { TypesAccepted.insert(type); }

    bool noConnectionSocket() {
        return (TypesAccepted.size() == 0);
    }

    // called from resize in Graph node.
    void setParameterCtrlBounds(juce::Rectangle<int> bound) {
        if (parameterController == nullptr) return;
        parameterController->setBoundForCtlr(bound);
    }

    // returns the parameter ctrl
    ParameterCtrl* getParameterCtrl() {
        return parameterController;
    }

    ~Socket() {}

    // if the node is on the input side then, this will return
    // a pointer to the node it is from.
    // this should only be called to check on for input sockets.
    // for the output sockets we will get the prev nodes by calling it on the
    // next node.
    void* getPrevNode() {

        switch (dir) {
            case (IN):
                return static_cast<void*>(from);
        }

        return nullptr;
    }

    void paint (juce::Graphics& g) override {
        g.fillAll(juce::Colours::red);
        if (parameterController != nullptr) {
            parameterController->paintAgain();
        }
    }

    //deleting the socket connection.
    void mouseDown(const juce::MouseEvent& event) override {
        if (event.mods.isRightButtonDown()) { // check if the connection is present.
            // Show context menu
//            juce::PopupMenu menu;
//            menu.addItem(1, "Delete Connection");
            //menu.showMenuAsync(juce::PopupMenu::Options(),
            //juce::ModalCallbackFunction::forComponent(deleteNodeCallback, this));
        }
    }

    // Add a custom parameter control
    // #### MOST OF THESE FUNCTIONS ARE MEANT TO BE USED ONLY BY dir `IN` Sockets. ####
    //
    void addMenuParameterControl() {
        if (parameterController != nullptr) {
            std::cout << "Resetting parameter control type is not allowed" << "\n";
            return;
        }

        parameterController = new MenuListParameterCtrl();
    }
    //
    void addSliderParameterControl(float from, float to, float default_) {
        if (parameterController != nullptr) {
            std::cout << "Resetting parameter control type is not allowed" << "\n";
            return;
        }

        parameterController = new NumericSliderParameterCtrl(from, to, default_);
    }

    // If the parameter control is set to MenuListParameterCtrl, this is used to add new types of menu options.
    // else it is not going to change anything and there is no reason to use this method.
    void addMenuItem(juce::String name) {

        MenuListParameterCtrl* control = dynamic_cast<MenuListParameterCtrl*>(parameterController);

        if (control == nullptr) {
            std::cout << "The parameter control is not a menu type, return" << "\n";
            return;
        }

        control->addItemToList(name);
    }

    // setting the type of an output node,
    // this is the type that is checked against the set of input types that can be
    // accepted from an input node.
    void setOutputType(SocketDataType a) {
        if (dir == direction::IN) {
            std::cout << "Trying to set the Output type of a Socket that has In direction, but why??" << "\n";
            return;
        }

        type = a;
    }

    // adding a new connection.
    void mouseDrag() {

    }

    bool isThisConnected() { return isConnected; }

    void resized() override {}

    bool isMust;

private:
    bool isConnected = false;
    void* from = nullptr, *to = nullptr;

    ParameterCtrl* parameterController;

    // Only set for Output nodes.
    SocketDataType type = NULLType;


    JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR(Socket)
};

I had the pointer to parameterCtrl with a unique_ptr till now but it did not make any difference, i get the same double free.

and also I’ve used OwnedArray to manage the whole Node which also resulted in a “double drop”(std::err by the program)

And also I am not so experienced with c++, so my deductions may be wrong.

The ParameterCtrl will be shown in the GraphNode but the Socket contains it, this sounds really weird and dumb so I am rewriting this part, but the question still remains the same.

4

AudioProcessorGraph nodes are reference counted and the graph will delete them when they’re removed from the graph

Rail

5

Thanks but these are my own ‘GraphNode’ they just inherit juce::Component.

6

You need to A) make the story of ParaneterCtrl virtual and B) could you please share one of the subclass that is triggering the error?

At this moment without unique_per the error above cannot happen anymore, because the dtor of Socket does not delete the controller. Could you share the new error?

7

After refractoring Socket to contain the ParameterCtrl i still get the same error at the same place.

==17439== Invalid free() / delete / delete[] / realloc()
==17439==    at 0x484BB6F: operator delete(void*, unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==17439==    by 0x1EBB86: std::default_delete<ParameterCtrl>::operator()(ParameterCtrl*) const (unique_ptr.h:85)
==17439==    by 0x1E931F: std::unique_ptr<ParameterCtrl, std::default_delete<ParameterCtrl> >::~unique_ptr() (unique_ptr.h:361)
==17439==    by 0x1E64B7: Socket::~Socket() (Socket.h:70)
==17439==    by 0x1E6507: Socket::~Socket() (Socket.h:70)
==17439==    by 0x1EEF1A: juce::ContainerDeletePolicy<Socket>::destroy(Socket*) (juce_ContainerDeletePolicy.h:54)
==17439==    by 0x1EC08A: juce::OwnedArray<Socket, juce::DummyCriticalSection>::deleteAllObjects() (juce_OwnedArray.h:815)
==17439==    by 0x1EC255: juce::OwnedArray<Socket, juce::DummyCriticalSection>::clearQuick(bool) (juce_OwnedArray.h:120)
==17439==    by 0x1E99E7: juce::OwnedArray<Socket, juce::DummyCriticalSection>::clear(bool) (juce_OwnedArray.h:109)
==17439==    by 0x1E732C: GraphNode::~GraphNode() (GraphNode.h:154)
==17439==    by 0x1E8259: Oscillator::~Oscillator() (Oscillator.h:61)
==17439==    by 0x1E8279: Oscillator::~Oscillator() (Oscillator.h:61)

all sub classes of ParameterCtrl :

class MenuListParameterCtrl : public juce::ComboBox,
                              public ParameterCtrl {
public:

    MenuListParameterCtrl() {
    }

    ~MenuListParameterCtrl() {}

    // ID's will be from 1 to the number of options in the list.
    void addItemToList(juce::String& name) {
        this->addItem(name, Index); Index++;
    }

    // we do not need a function to remove this. \\

    int getSelectedItemsIndex() {
        return getSelectedId(); // returns the index of the currently selected id.
    }

    void paintAgain() override {
        repaint();
    }

    // for menu it sends 0.5 less than the menu option ID, to remove precision errors.
    // use less than 1 less than 2 and so on.
    float getValue() override {
        return ((float)(getSelectedId())-0.5);
    }

    void setBoundForCtlr(juce::Rectangle<int> bound) {
        setBounds(bound);
    }

    int getType() override { return 1; };

private:

    int Index = 1;

};

// value is always floating.
class NumericSliderParameterCtrl : public ParameterCtrl,
                                   public juce::TextEditor {
public:

    NumericSliderParameterCtrl(float from, float to, float default_) {
        this->from = from;
        this->to = to;
        this->value = default_;

    }

    void paintAgain() override {
        repaint();
    }


    float getValue() override {
        return value;
    }

    ~NumericSliderParameterCtrl() {};

    void setBoundForCtlr(juce::Rectangle<int> bound) {
        setBounds(bound);
    }

    int getType() override { return 2; };


private:

    float from, to, value;

};

The error is occurring when auto destructing “std::unique_ptr<ParameterCtrl*>” in Socket class.

8

Isn’t this supposed to be std::unique_ptr<ParameterCtrl>?

9

my mistake, yes it is :

std::unique_ptr<ParameterCtrl> parameterController;
10

OT: You don’t need to supply default destructors. If you do, write

~MenuListParameterCtrl() = default;

If you have virtual functions, you should make the destructor virtual (which is the exception to the above, here you write a default virtual destructor).

On topic:

Your whole life could be better by choosing aggregation over inheritance. If you need functionality from a class, add a member of that type.

The classes you mentioned are already so entangled, that without drawing a chart you and other readers are likely to get lost.

11

Thanks for the tip. In my case I need to add this as a child component. so I thought we could implement custom types any time without any code modification in the API. but I can see how it can be done through aggregation though.

12

rewrote ParameterCtrl from @daniel advice. Did not find the reason behind the main issue(double free) but it is no more a problem now. Everything’s working as expected.

// invisible container that contains other controls.
class ParameterCtrl : public juce::Component {
public:

    ParameterCtrl() {
        setVisible(false);
    }

    ~ParameterCtrl() override {};

    float getValue() {
        return 0.0; // TODO
    };

    int getType() {
        return parameterType;
    }

    void createNewMenu() {
        if (parameterType != -1) {
            std::cout << "Trying to rewrite the parameterCtrl type, not possible" << "\n";
            return;
        }

        menuList.reset(new juce::ComboBox);
        parameterType = 1;
    }

    void addItemToList(juce::String name) {
        if (menuList.get()) menuList.get()->addItem(name, index); index++;
    }

    void createTextEditor(float from , float to , float val) {
        if (parameterType != -1) {
            std::cout << "Trying to rewrite the parameterCtrl type, not possible" << "\n";
            return;
        }

        textEditor.reset(new juce::TextEditor);
        this->from = from;
        this->to = to;
        this->value = val;

        parameterType = 2;
    }


    // called from the socket when ready to make it visible.
    void update() {
        // checking if it is not nullptr.
       // and only one or none of them can be true at a time.
        if (menuList.get()) {
            menuList.get()->setBounds(getLocalBounds().reduced(4));
            addAndMakeVisible(menuList.get());
            // do not ask me if you did not even add one option, dude.
            menuList.get()->setSelectedItemIndex(1, juce::sendNotificationSync);
        } else if (textEditor.get()) {
            textEditor.get()->setBounds(getLocalBounds().reduced(4));
            addAndMakeVisible(textEditor.get());
            textEditor.get()->setText(juce::String(value));
        }
    }

    // lock and unlock while reading the values.
    void lock() { mutex.lock(); }

    void unlock() { mutex.unlock(); }



    void paint(juce::Graphics& g) override {}
    void resized() override {}


private:
    // This is to lock this when others are reading or setting it.
    std::mutex mutex;

    int parameterType = -1; // means no control.
    // ^ 1 for menu and two for the text input.
    // 2 for the float input

    std::unique_ptr<juce::ComboBox> menuList;
    int index = 1;

    std::unique_ptr<juce::TextEditor> textEditor;
    float from, to, value;


    JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (ParameterCtrl)
};