Closing plugin window does not dismiss AlertWindow; memory leaks [Solved]

I’m not sure if this is expected behavior, but I don’t see how it can be.

If you launch an AlertWindow in your plugin (async with autoDelete), then delete the plugin (right-click) with the Alert open, you hit an assert. If you then close the AudioPluginHost, you get memory leaks.

I am calling juce::ModalComponentManager::getInstance()->cancelAllModalComponents(); in the AudioProcessorEditor destructor.

I’m testing this in AudioPluginHost, Juce 6.1.6. MacOS Mojave.

A demo project is attached:

AlertTest.zip (26.7 KB)

Steps to reproduce:

  • debug the TestAlert project into AudioPluginHost.
  • after APH is launched, add a instance of your company > TestAlert VST3. Open the editor.
  • click the Alert button - alert pops up.
  • right click on TestAlert plugin and “delete filter”.
  • Assert hits :
JUCE v6.1.6
JUCE Assertion failure in juce_DeletedAtShutdown.cpp:82
  • Continue in the debugger.
  • Quit AudioPluginHost, answer “Discard” to changes.
    Memory leaks:
*** Leaked objects detected: 1 instance(s) of class TimerThread
JUCE Assertion failure in juce_LeakedObjectDetector.h:92
*** Leaked objects detected: 1 instance(s) of class Thread
JUCE Assertion failure in juce_LeakedObjectDetector.h:92
*** Leaked objects detected: 3 instance(s) of class WaitableEvent
JUCE Assertion failure in juce_LeakedObjectDetector.h:92
… etc

Thought this might be related, but it doesn’t seem to apply:

Here’s the relevant source:

//==============================================================================
AlertTestAudioProcessorEditor::AlertTestAudioProcessorEditor (AlertTestAudioProcessor& p)
    : AudioProcessorEditor (&p), audioProcessor (p)
{
    // Make sure that before the constructor has finished, you've set the
    // editor's size to whatever you need it to be.
    setSize (800, 500);
    
    addAndMakeVisible(alertButton);
    alertButton.onClick = [this] {
 
        auto* aw = new juce::AlertWindow("This is a test...", {}, juce::AlertWindow::InfoIcon, this);
        
        aw->addButton ("OK", 0, juce::KeyPress (juce::KeyPress::returnKey));

        aw->enterModalState (true,
                             juce::ModalCallbackFunction::create
                             ([this] (int result)
                              {
                                  if (result == 1) // OK
                                      DBG ("result = 1");
                              }), true);
    };
}

AlertTestAudioProcessorEditor::~AlertTestAudioProcessorEditor()
{
    juce::ModalComponentManager::getInstance()->cancelAllModalComponents();
}

//==============================================================================
void AlertTestAudioProcessorEditor::paint (juce::Graphics& g)
{
    // (Our component is opaque, so we must completely fill the background with a solid colour)
    g.fillAll (getLookAndFeel().findColour (juce::ResizableWindow::backgroundColourId));

    g.setColour (juce::Colours::white);
    g.setFont (15.0f);
    g.drawFittedText ("Hello World!", getLocalBounds(), juce::Justification::centred, 1);
}

void AlertTestAudioProcessorEditor::resized()
{
    // This is generally where you'll want to lay out the positions of any
    // subcomponents in your editor..
    alertButton.setBounds(20, 20, 100, 30);
}

I can reproduce the problem, but at the moment I’m not sure there’s much we can do to resolve it.

Calling cancelAllModalComponents doesn’t seem to immediately cancel and destroy any currently modal components - instead, the destruction happens in a subsequent event on the message thread.

The problem seems to be that the alert window component and its associated Peer are still alive at the point when the modal component manager is destroyed. However, the destruction order of JUCE’s singletons isn’t well-defined. This means that, at the point that the alert window destructor is called, the Leak Detector singleton, Timer Thread singleton, and other vital objects may already have been destroyed. This is why you were seeing leak detector failures - the leak detector is checking for leaks before the alert window is destroyed.

The solution is to ensure that all components are completely destroyed in your editor’s destructor. I can work around the problem in your example code simply by holding a unique_ptr to the alert window in the editor, and setting deleteWhenDismissed to false:

std::unique_ptr<juce::Component> modalComponent;

explicit AudioPluginAudioProcessorEditor (AudioPluginAudioProcessor& p)
    : AudioProcessorEditor (&p), processorRef (p)
{
    setSize (800, 500);

    addAndMakeVisible (alertButton);

    alertButton.onClick = [this]
    {
        auto aw = std::make_unique<juce::AlertWindow> ("This is a test...",
                                                       juce::String{},
                                                       juce::MessageBoxIconType::InfoIcon,
                                                       this);

        aw->addButton ("OK", 0, juce::KeyPress (juce::KeyPress::returnKey));

        aw->enterModalState (true, juce::ModalCallbackFunction::create ([ref = SafePointer<AudioPluginAudioProcessorEditor> (this)] (int result)
        {
            if (ref == nullptr)
                return;

            ref->modalComponent = nullptr;

            if (result == 1)
                DBG ("result = 1");
        }));

        modalComponent = std::move (aw);
    };
}

There’s another reason that I’d recommend against using your current approach: the modal component manager is shared between all instances of the plugin. If your user opens two copies of your plugin’s editor, then opens a modal dialog in each plugin window, closing either plugin window will dismiss both of the modal dialogs. This is probably unexpected - the user would probably only expect the modal dialog created by the closed editor to be removed.

2 Likes

Thanks for that explanation! I had started looking into storing a pointer to the alert as a solution, so it’s good to see that approach confirmed. I can confirm your solutions works.

Coding question: assuming you wanted to plan for the undesirable yet possible case of two or more overlapping AlertWindows, what would you recommend using? An array of std::unique_ptr, or OwnedArray<AlertWindow>? I’m not quite sure of the difference…OwnedArray seems easier to use…

I’d normally suggest using a vector<unique_ptr<Component>>, but I don’t have a really strong argument for either approach.

1 Like

My 2 cents here: as a rule of thumb, I generally prefer a vector of unique_ptr whenever possible.

A good exception to that rule is when I need to extract an item from the collection without deleting it: OwnedArray implements that with removeAndReturn(), whereas there’s not an easy way to do it with a vector of unique_ptr (please correct me if I’m wrong!)