VST crashing with PopupMenu in Windows

Hi all,

I’m having a strange, erratic bug in my latest Windows plugin (demo can be grabbed from ValhallaRoom 1.5.1 Released for OSX and Windows. Electric Blue GUI. - Valhalla DSP). I’m using a PopupWindow for the preset menu. Things work great in OSX, but on Windows, if you do a lot of rapid browsing in the PopupMenu, and open up PopupMenus from the main PopupMenu, it crashes the DAW.

Here’s the call stack:

[code] ValhallaRoom.dll!juce::WeakReferencejuce::Component::SharedPointer::get() Line 137 + 0xa bytes C++
ValhallaRoom.dll!juce::WeakReferencejuce::Component::Master::operator()(juce::Component * const object) Line 180 + 0x12 bytes C++
ValhallaRoom.dll!juce::Component::getWeakReference() Line 448 C++
ValhallaRoom.dll!juce::WeakReferencejuce::Component::WeakReferencejuce::Component(juce::Component * const object) Line 92 + 0x37 bytes C++
ValhallaRoom.dll!juce::Component::BailOutChecker::BailOutChecker(juce::Component * const component) Line 3031 + 0x26 bytes C++
ValhallaRoom.dll!juce::Component::internalHierarchyChanged() Line 1510 + 0xf bytes C++
ValhallaRoom.dll!juce::Component::setAlwaysOnTop(const bool shouldStayOnTop) Line 902 C++
ValhallaRoom.dll!juce::DropShadower::updateShadows() Line 255 C++
ValhallaRoom.dll!juce::DropShadower::componentVisibilityChanged(juce::Component & __formal) Line 176 C++
ValhallaRoom.dll!juce::ListenerList<juce::ComponentListener,juce::Array<juce::ComponentListener * __ptr64,juce::DummyCriticalSection> >::callChecked<juce::Component::BailOutChecker,juce::Component & __ptr64>(const juce::Component::BailOutChecker & bailOutChecker, void (juce::Component &)* callbackFunction, juce::Component & param1) Line 181 + 0x16 bytes C++
ValhallaRoom.dll!juce::Component::sendVisibilityChangeMessage() Line 537 + 0x24 bytes C++
ValhallaRoom.dll!juce::Component::setVisible(bool shouldBeVisible) Line 511 C++
ValhallaRoom.dll!juce::PopupMenu::window::showSubMenuFor(juce::PopupMenu::ItemComponent * const childComp) Line 1041 C++
ValhallaRoom.dll!juce::PopupMenu::window::timerCallback() Line 554 C++
ValhallaRoom.dll!juce::InternalTimerThread::callTimers() Line 138 + 0x10 bytes C++
ValhallaRoom.dll!juce::InternalTimerThread::handleMessage(const juce::Message & __formal) Line 155 C++
ValhallaRoom.dll!juce::MessageManager::deliverMessage(juce::Message * const message) Line 116 + 0x15 bytes C++
ValhallaRoom.dll!juce::MessageManager::dispatchNextMessageOnSystemQueue(const bool returnIfNoPendingMessages) Line 164 C++
ValhallaRoom.dll!juce::MessageManager::runDispatchLoopUntil(int millisecondsToRunFor) Line 148 + 0x1d bytes C++
ValhallaRoom.dll!juce::ModalComponentManager::runEventLoopForCurrentComponent() Line 280 + 0x12 bytes C++
ValhallaRoom.dll!juce::Component::runModalLoop() Line 1553 C++

ValhallaRoom.dll!juce::PopupMenu::showWithOptionalCallback(const juce::PopupMenu::Options & options, juce::ModalComponentManager::Callback * const userCallback, const bool canBeModal) Line 1516 + 0x21 bytes C++
ValhallaRoom.dll!juce::PopupMenu::showAt(juce::Component * componentToAttachTo, const int itemIdThatMustBeVisible, const int minimumWidth, const int maximumNumColumns, const int standardItemHeight, juce::ModalComponentManager::Callback * callback) Line 1582 + 0x1d bytes C++
ValhallaRoom.dll!ValhallaRoomEditor::doPatchMenu() Line 141 + 0x38 bytes C++
ValhallaRoom.dll!ValhallaRoomEditor::buttonClicked(juce::Button * buttonThatWasClicked) Line 597 C++
ValhallaRoom.dll!juce::ListenerList<juce::Button::Listener,juce::Array<juce::Button::Listener * __ptr64,juce::DummyCriticalSection> >::callChecked<juce::Component::BailOutChecker,juce::Button * __ptr64>(const juce::Component::BailOutChecker & bailOutChecker, void (juce::Button ) callbackFunction, juce::Button * param1) Line 181 + 0x16 bytes C++
ValhallaRoom.dll!juce::Button::sendClickMessage(const juce::ModifierKeys & modifiers) Line 384 + 0x24 bytes C++
ValhallaRoom.dll!juce::Button::internalClickCallback(const juce::ModifierKeys & modifiers) Line 326 C++
ValhallaRoom.dll!juce::Button::mouseUp(const juce::MouseEvent & e) Line 441 C++
ValhallaRoom.dll!juce::Component::internalMouseUp(juce::MouseInputSource & source, const juce::Point & relativePos, const juce::Time & time, const juce::ModifierKeys & oldModifiers) Line 2483 C++
ValhallaRoom.dll!juce::MouseInputSourceInternal::sendMouseUp(juce::Component * const comp, const juce::Point & screenPos, const juce::Time & time) Line 134 + 0x67 bytes C++
ValhallaRoom.dll!juce::MouseInputSourceInternal::setButtons(const juce::Point & screenPos, const juce::Time & time, const juce::ModifierKeys & newButtonState) Line 166 + 0x4f bytes C++
ValhallaRoom.dll!juce::MouseInputSourceInternal::handleEvent(juce::ComponentPeer * const newPeer, const juce::Point & positionWithinPeer, const juce::Time & time, const juce::ModifierKeys & newMods) Line 279 + 0x19 bytes C++
ValhallaRoom.dll!juce::MouseInputSource::handleEvent(juce::ComponentPeer * peer, const juce::Point & positionWithinPeer, const __int64 time, const juce::ModifierKeys & mods) Line 528 + 0x66 bytes C++
ValhallaRoom.dll!juce::ComponentPeer::handleMouseEvent(const int touchIndex, const juce::Point & positionWithinPeer, const juce::ModifierKeys & newMods, const __int64 time) Line 108 C++
ValhallaRoom.dll!juce::Win32ComponentPeer::doMouseEvent(const juce::Point & position) Line 1397 C++
ValhallaRoom.dll!juce::Win32ComponentPeer::doMouseUp(const juce::Point & position, const unsigned int64 wParam) Line 1496 C++
ValhallaRoom.dll!juce::Win32ComponentPeer::peerWindowProc(HWND
* h, unsigned int message, unsigned __int64 wParam, int64 lParam) Line 2051 + 0x47 bytes C++
ValhallaRoom.dll!juce::Win32ComponentPeer::windowProc(HWND
* h, unsigned int message, unsigned __int64 wParam, __int64 lParam) Line 1969 + 0x23 bytes C++
[/code]

The actual line that displays the menu in my own code is

const int result = menu.showAt (presetsSelect);

I have a feeling that this might have something to do with modal state, but I HAVE NO IDEA WHAT THAT MEANS. I am just using the PopupMenu, I have no idea why this is crashing, and I have a bunch of people complaining about it in a bunch of forums, so my brain is a bit frazzled. Any assistance is greatly appreciated. Example code, or pointers to example code for fixes, is always welcome (DSP guy here - not great at the deeper C++ stuff, but I can rattle off a few dozen allpass topologies that are useful).

Thanks,

Sean Costello

Yep.

Don’t run modal loops in plugins!!

If you’re just showing a dialog box, call showDialog() and not showModalDialog(). If you’re using a PopupMenu, make sure you always provide a callback object.

Is there a way for Juce to know, and throw a jassert when a modal operation is attempted in a plugin?

Yes… that’d be very quite easy to do, just a one-liner in the juce_Component.cpp:

int Component::runModalLoop() { jassert (JUCEApplication::isStandaloneApp());

Not sure if I want to add it though - I’d imagine that a lot of people would start getting assertions, and in some cases may be doing in intentionally.

[quote=“jules”]Yep.

Don’t run modal loops in plugins!!

If you’re using a PopupMenu, make sure you always provide a callback object.[/quote]

Example code for this?

Thanks,

Sean

Lots of examples in the codebase + demo. Just search for “showMenuAsync”

I did that. No results. Got something else to search for?

It’s used all over the place… you’ve probably not got the latest version?

No, I last updated in March or so. Not a good idea to go to the bleeding edge that close to a release (the Mac version came out March 23rd), especially since you moved the plugin wrapper files. Mumble, grumble.

Anyway, just downloaded the tip with the Introjucer (no mumble grumble there - I love the GIT update feature in the Introjucer), and it shows up all over the place. I’ll take a look in the demo and see how it is used.

Thanks,

Sean Costello

Alrighty, I updated my PopupMenu code to use a callback object. Now I get a different crash!

Here’s the crash log:

> ValhallaRoom.dll!juce::WeakReference<juce::Component>::SharedPointer::get() Line 21597 + 0xa bytes C++ ValhallaRoom.dll!juce::WeakReference<juce::Component>::Master::operator()(juce::Component * const object) Line 180 + 0x12 bytes C++ ValhallaRoom.dll!juce::Component::getWeakReference() Line 448 C++ ValhallaRoom.dll!juce::WeakReference<juce::Component>::WeakReference<juce::Component>(juce::Component * const object) Line 21553 + 0x37 bytes C++ ValhallaRoom.dll!juce::Component::BailOutChecker::BailOutChecker(juce::Component * const component) Line 3031 + 0x26 bytes C++ ValhallaRoom.dll!juce::Component::internalHierarchyChanged() Line 1510 + 0xf bytes C++ ValhallaRoom.dll!juce::Component::setAlwaysOnTop(const bool shouldStayOnTop) Line 902 C++ ValhallaRoom.dll!juce::DropShadower::updateShadows() Line 255 C++ ValhallaRoom.dll!juce::DropShadower::componentVisibilityChanged(juce::Component & __formal) Line 176 C++ ValhallaRoom.dll!juce::ListenerList<juce::ComponentListener,juce::Array<juce::ComponentListener * __ptr64,juce::DummyCriticalSection> >::callChecked<juce::Component::BailOutChecker,juce::Component & __ptr64>(const juce::Component::BailOutChecker & bailOutChecker, void (juce::Component &)* callbackFunction, juce::Component & param1) Line 181 + 0x16 bytes C++ ValhallaRoom.dll!juce::Component::sendVisibilityChangeMessage() Line 537 + 0x24 bytes C++ ValhallaRoom.dll!juce::Component::setVisible(bool shouldBeVisible) Line 511 C++ ValhallaRoom.dll!juce::PopupMenu::Window::showSubMenuFor(juce::PopupMenu::ItemComponent * const childComp) Line 1042 C++ ValhallaRoom.dll!juce::PopupMenu::Window::timerCallback() Line 554 C++ ValhallaRoom.dll!juce::InternalTimerThread::callTimers() Line 140 + 0x10 bytes C++ ValhallaRoom.dll!juce::InternalTimerThread::handleMessage(const juce::Message & __formal) Line 157 C++ ValhallaRoom.dll!juce::MessageManager::deliverMessage(juce::Message * const message) Line 115 + 0x15 bytes C++ ValhallaRoom.dll!juce::juce_MessageWndProc(HWND__ * h, juce::Message * const message, const unsigned __int64 wParam, const __int64 lParam) Line 72 C++

In order to reproduce this, I have to really move my mouse all over the popup menu, and submenus, like a crazy guy. However, other users reported the crash with normal usage.

So, now that the dread modal loop has been removed from the PopupMenu, any idea what might be causing this crash?

Thanks,

Sean Costello

Well… I think I can see what’s happening, though impossible to tell what you’re doing to trigger it!

By the looks of it, you’ve managed to hit a situation where the Component class needs to be doing a bit of extra testing to make sure that things aren’t being deleted unexpectedly - try this, and see if it helps:

[code]void Component::setAlwaysOnTop (const bool shouldStayOnTop)
{
if (shouldStayOnTop != flags.alwaysOnTopFlag)
{
BailOutChecker checker (this);

    flags.alwaysOnTopFlag = shouldStayOnTop;

    if (isOnDesktop())
    {
        ComponentPeer* const peer = getPeer();

        jassert (peer != nullptr);
        if (peer != nullptr)
        {
            if (! peer->setAlwaysOnTop (shouldStayOnTop))
            {
                // some kinds of peer can't change their always-on-top status, so
                // for these, we'll need to create a new window
                const int oldFlags = peer->getStyleFlags();
                removeFromDesktop();
                addToDesktop (oldFlags);
            }
        }
    }

    if (shouldStayOnTop && ! checker.shouldBailOut())
        toFront (false);

    if (! checker.shouldBailOut())
        internalHierarchyChanged();
}

}[/code]

Thanks, Jules. I will try the code modification.

BTW, I pasted some “static” code into my preset code last night, that brings up a fixed menu, instead of one that is generated dynamically from the file director. I couldn’t make it crash. So the PopupMenu by itself is probably not the issue. I’m working on separating the construction of the menu directory from the PopupMenu itself.

Sean Costello

I made the change that Jules recommended, as well as cleaning up my own preset/PopupMenu code. In the past, the PopupMenu was populated with presets each time it was brought up, which means that I was probably going to the file structure a lot. I changed the code, such that the items in the PopupMenu are only updated at initialization time, and when a new preset is saved.

So, I’ve got a new crash log for you. This is with the changes to Component as suggested above:

> ValhallaRoom.dll!juce::OwnedArray<juce::Component,juce::DummyCriticalSection>::getUnchecked(const int index) Line 124 + 0x71 bytes C++ ValhallaRoom.dll!juce::DropShadower::updateShadows() Line 255 + 0x1b bytes C++ ValhallaRoom.dll!juce::DropShadower::componentVisibilityChanged(juce::Component & __formal) Line 176 C++ ValhallaRoom.dll!juce::ListenerList<juce::ComponentListener,juce::Array<juce::ComponentListener * __ptr64,juce::DummyCriticalSection> >::callChecked<juce::Component::BailOutChecker,juce::Component & __ptr64>(const juce::Component::BailOutChecker & bailOutChecker, void (juce::Component &)* callbackFunction, juce::Component & param1) Line 181 + 0x16 bytes C++ ValhallaRoom.dll!juce::Component::sendVisibilityChangeMessage() Line 537 + 0x24 bytes C++ ValhallaRoom.dll!juce::Component::setVisible(bool shouldBeVisible) Line 511 C++ ValhallaRoom.dll!juce::PopupMenu::Window::showSubMenuFor(juce::PopupMenu::ItemComponent * const childComp) Line 1042 C++ ValhallaRoom.dll!juce::PopupMenu::Window::timerCallback() Line 554 C++ ValhallaRoom.dll!juce::InternalTimerThread::callTimers() Line 140 + 0x10 bytes C++ ValhallaRoom.dll!juce::InternalTimerThread::handleMessage(const juce::Message & __formal) Line 157 C++ ValhallaRoom.dll!juce::MessageManager::deliverMessage(juce::Message * const message) Line 115 + 0x15 bytes C++ ValhallaRoom.dll!juce::juce_MessageWndProc(HWND__ * h, juce::Message * const message, const unsigned __int64 wParam, const __int64 lParam) Line 72 C++

I was able to get the crash fairly quickly on occasion - just scrolling through a few PopupMenu items.

It is worth noting that the PopupMenu code has always been rock solid on OSX. It is just Windows that is giving me grief.

Thanks for your help,

Sean Costello

Well, I’m still baffled as to what you’re doing to trigger the problem, but here’s some more safety-checking to try, which should go in DropShadower::updateShadows()

[code] if (shadowWindows.size() >= 4)
{
const int x = owner->getX();
const int y = owner->getY() - shadowEdge;
const int w = owner->getWidth();
const int h = owner->getHeight() + shadowEdge + shadowEdge;

        for (int i = shadowWindows.size(); --i >= 0;)
        {
            WeakReference<Component> sw (shadowWindows.getUnchecked(i));
            
            if (sw == nullptr)
                return;

            sw->setAlwaysOnTop (owner->isAlwaysOnTop());

            if (sw == nullptr)
                return;
            
            switch (i)
            {
                case 0: sw->setBounds (x - shadowEdge, y, shadowEdge, h); break;
                case 1: sw->setBounds (x + w, y, shadowEdge, h); break;
                case 2: sw->setBounds (x, y, w, shadowEdge); break;
                case 3: sw->setBounds (x, owner->getBottom(), w, shadowEdge); break;
                default: break;
            }
        }
    }

[/code]

The stack trace looks like the drop-shadow windows are doing something that’s deleting a component in the middle of an operation, but hard to tell much more, and I can’t see any obvious mistakes. This change is probably a good safety feature anyway, and might be what’s needed here.

Early morning update: I integrated Jules’ DropShadower update, and have been trying to make the plugin crash. It seems stable. Next step is to send it out to beta testers and see if they can crash it.

Thanks for your help,

Sean

Thanks Sean, though I wish I knew how you were managing to get it into that state! Must be a very unusual situation for it not to have happened before.

It is one of my many gifts!

I’m back. sigh

One of my users is reporting that my plugin is trying to install a “global hook”:

[quote=“V’ger”]I really like the GUI. The big bold knobs and sliders are very businesslike.

Agree preset path is a mess, especially when they are not even in \app data\ValhallaRoom, but just save to your DAW native and backup should be less of a chore.

But what’s with the global hook on load Sean?


Not that I don’t trust you, but obviously don’t want anything opened unnecessary as a rule.[/quote]

Is this something that might have been introduced with the bug fixes, or in the latest tip of Juce?

If not, if you have any suggestions what this is about (installer?), they are very welcome.

Jules, I appreciate all of your help on this issue. This cross-platform stuff is HARD, and I know you have put a lot of work into it. If I feel like throwing in the towel on Windows, I can’t imagine what you must think about the whole thing.

Thanks,

Sean Costello

Sigh.

The plugin stuff uses a mouse wheel hook, but that’s been in there for a long time (well, at least a few months), and I’ve never heard of any problems before. Maybe some kind of virus checker has just started catching that sort of behaviour.

Looking through the MS docs again, I guess that the warning might be avoided if it uses a different hook type and is more specific about the thread that it affects, e.g. by changing this line in juce_VST_Wrapper.cpp:

mouseWheelHook = SetWindowsHookEx (7 /*WH_MOUSE*/, mouseWheelHookCallback, (HINSTANCE) PlatformUtilities::getCurrentModuleInstanceHandle(), GetCurrentThreadId());

…but I’ve no way of testing that. And changing the WH_MOUSE_LL to a WH_MOUSE hook may not work as well. Can’t think of anything else though, other than removing the hook and not handling the mouse wheel properly.

Did we lose some replies in this thread? I know that The Vinn had a response about how it was probably a particular virus checker that was running into the global hook issue, and I had said that I was less inclined to change the code base for 1 user. Then Jules responded with a request that I try the code change, and I said I would in a few days. I doubt that I hallucinated all that - if I did, my insanity is proving to be pretty boring.

Sean Costello