Popup menu crashes on exit

If a TextEdit is in progress with a right click PopupMenu visible and someone exits the application this causes an access violation and crash.

I first found the bug by having a right click menu on my plugin. As a workaround I tried adding the menu to it’s own window via the addToDesktop method but the menu still remains visible and fails to receive the focusLost virtual method when it has lost focus by someone clicking on the host.

Any ideas?

It’s the toughest problem I’ve had with plugins so far. I think the only workaround might be some kind of special bodge that uses a normal OS modal state to block the rest of the app from getting events while the menu is there, but I tried that a while ago and hit all sorts of other stupid problems…

I have sorted out the focus problem, so the following system seems to be working pretty well.

class DesktopPopupMenu;

class DesktopPopupMenuListener
{
public:
	virtual ~DesktopPopupMenuListener () {};
	virtual void menuAccepted (DesktopPopupMenu* menu, const int item) = 0;
	virtual void menuRejected (DesktopPopupMenu* menu) = 0;
};

class DesktopPopupMenu
	: public PopupMenu
	, public ComponentListener
{
public:
	DesktopPopupMenu (const tchar* name, DesktopPopupMenuListener* listener);
	virtual ~DesktopPopupMenu ();
	const String& getName () const;
	void showAt (int screenX, int screenY);
	void showAt (Component* anchor);

protected:
	void handleMenuExit ();
	virtual void componentVisibilityChanged (Component& component);

	String name;
	DesktopPopupMenuListener* listener;
	Component* menuwindow;
};


DesktopPopupMenu::DesktopPopupMenu (const tchar* name_, DesktopPopupMenuListener* listener_)
: PopupMenu ()
, name (name_)
, listener (listener_)
, menuwindow (0)
{
	jassert (listener);
}

DesktopPopupMenu::~DesktopPopupMenu ()
{
	handleMenuExit ();
}

const juce::String& DesktopPopupMenu::getName () const
{
	return name;
}

void DesktopPopupMenu::showAt (Component* anchor_)
{
	Rectangle menurect = anchor_->getBounds ();
	menurect.setPosition (anchor_->getScreenX(), anchor_->getScreenY());
	showAt (menurect.getX (), menurect.getY () + menurect.getHeight ());
}
void DesktopPopupMenu::showAt (int screenX_, int screenY_)
{
	menuwindow = createMenuComponent (screenX_, screenY_, 1, 1, 0, 0, 0, false, 0, 0, 0, 0);
	menuwindow->setWantsKeyboardFocus (true);
	menuwindow->setVisible (true);
	menuwindow->addComponentListener (this);
	menuwindow->addToDesktop (0, 0);
	menuwindow->enterModalState ();
};

void DesktopPopupMenu::handleMenuExit ()
{
	if (menuwindow)
	{
		menuwindow->removeComponentListener (this);
		int item = menuwindow->getAndRemoveModalReturnValue ();
		delete menuwindow;
		menuwindow = 0;
		if (item > 0)
		{
			listener->menuAccepted (this, item);
		}
		else
		{
			listener->menuRejected (this);
		}
	}
}

void DesktopPopupMenu::componentVisibilityChanged (Component& component_)
{
	if (!component_.isVisible ())
	{
		handleMenuExit ();
		delete this;
	}
}

Below is an example usage of the DesktopPopupMenu, then menu removes itself so you just have to match whatever you called the menu in your callback, that way you can have multiple different menus doing different sets of commands through the same interface and not worry about trying to keeps the ids different:

void TheGlueEditorComponent::PopupPresetMenu ()
{
	DesktopPopupMenu* menu = new DesktopPopupMenu (T("PresetMenu"), this);
	menu->addItem (Menu_Rename, T("Rename"), true);
	menu->addSeparator ();
	menu->addItem (Menu_Load, T("Load..."), true);
	menu->addItem (Menu_Save, T("Save..."), true);
	menu->addSeparator ();
	menu->addItem (Menu_Copy, T("Copy"), true);
	menu->addItem (Menu_Paste, T("Paste"), true);
	if (buttonab && buttonab->getToggleState ())
	{
		menu->addItem (Menu_Duplicate, T("Copy B->A"), true);
	}
	else
	{
		menu->addItem (Menu_Duplicate, T("Copy A->B"), true);
	} 
	menu->addSeparator ();
	menu->addItem (Menu_About, T("About..."), true);

	menu->showAt (buttonpreset);
}

void TheGlueEditorComponent::menuAccepted (DesktopPopupMenu* menu, const int item)
{
	if (menu->getName ().equalsIgnoreCase (T("PresetMenu")))
	{
		switch (item)
		{
		case Menu_None     : break;
		case Menu_Rename   : CommandRename (); break;
		case Menu_Load     : CommandLoad (); break;
		case Menu_Save     : CommandSave (); break;
		case Menu_Copy     : CommandCopy (); break;
		case Menu_Paste    : CommandPaste (); break;
		case Menu_Duplicate: CommandDuplicate (); break;
		case Menu_About    : CommandAbout (); break;
		}
	}
}

void TheGlueEditorComponent::menuRejected (DesktopPopupMenu* menu)
{
	// do nothing
}

Seems like a cunning plan!

Nice, but this is only a kind of workaround for a problem that is not specific to PopupMenu’s only. When there’s a modal dialog opened, the same problem arises. So one would also need to apply it to that.

this might help too:


int Component::getAndRemoveModalReturnValue ()
{
    const int modalIndex = modalComponentReturnValueKeys.indexOf (this);
    int returnValue = 0;

    if (modalIndex >= 0)
    {
        modalComponentReturnValueKeys.remove (modalIndex);
        returnValue = modalReturnValues.remove (modalIndex);
    }

    modalComponentStack.removeValue (this);

	return returnValue;
}

Revisiting this issue because 10 years later, I still see this in our JUCE plugins. If the user opens a popup menu, then closes the project/plugin, the DAW crashes.

Is there any way to get around this, other than what is described here? (I would think that the owner of the menu could close it somehow if it is open when the class is being destroyed, but I tried something like that with VSTGUI for open dialogs, and didn’t have much luck.)

1 Like

@HowardAntares in our plugins we call the static method PopupMenu::dismissAllActiveMenus() in the destructor of our AudioProcessorEditor. Does this work for you?

2 Likes

Crap. Spoke too soon. It didn’t crash the first time I tested it, but three subsequent tests all crashed. :frowning:

After more testing, it seems random. Sometimes it crashes, sometimes it does not.

1 Like

Aw man :disappointed: where is the crash happening? Is the open PopupMenu trying to reference something that has been deleted?

This is the stack trace (redacted):

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.XXX.XXX       	0x000000013e2f69ac juce::Array<juce::AudioProcessorListener*, juce::DummyCriticalSection, 0>::size() const + 12 (juce_Array.h:246)
1   com.XXX.XXX       	0x000000013e2f6553 juce::AudioProcessor::updateHostDisplay() + 35 (juce_AudioProcessor.cpp:568)
2   com.XXX.XXX       	0x000000013e298d5c XXXPluginAudioProcessorEditor::setStateAsDirty() + 28 (PluginEditor.h:77)
3   com.XXX.XXX       	0x000000013e2989ae XXXPluginAudioProcessorEditor::onSettingsButtonPressed() + 926 (PluginEditor.cpp:749)
...

I’ll see if I can step through in the debugger and get any more, but given that the component that opened the menu has been destroyed, I’m not hopeful.

EDIT: Looks like the added call to setStateAsDirty() is causing this problem. I added that to make sure that DAWs know something might have changed in the plugin when changing menu selections, so that it will prompt to save upon closing. Maybe I should only do that if one of the valid selections was made?

This may not matter if the PopupMenu isn’t actually trying to reference that component, IIRC.

The call to updateHostDisplay() happening sounds strange given the situation. Shouldn’t nothing happen if no option was chosen from the menu?

Ah, you read my mind :slight_smile:

1 Like

Yes, that does it, apparently. Ten times in a row, no crash any more. :slight_smile: Thanks!

1 Like

You’re not calling the show() method, are you?
The only safe way to show a popup menu in a plugin is with showMenuAsync()

If you want to be sure, just set JUCE_MODAL_LOOPS_PERMITTED=0 in your project, and it should be impossible to call anything that causes these shutdown modal loop problems.

I suppose this is obvious now I think about it, but it never occurred to me in 18 months of almost full time JUCE plugin development.

I can see a lot of value in a “list of dos and don’ts for JUCE plugin developer noobs” tutorial if you ever feel so inclined.

Yes, I am calling show(). I had no idea that this was a no-no. We stopped using modal dialogs, but I had not been aware that a menu could cause similar problems. Using dismissAllActiveMenus() in my editor’s destructor, and not calling any other code if I don’t get one of my expected responses back, seems to work, though. Should I rewrite my menu using the async method instead?

[EDIT] Changed my code to use showMenuAsync(), with a lambda for the callback, allowing my code to have minimal changes. Seems to work perfectly. Thanks!

4 Likes