Issue with Guitarix plugins confirmed to be a bug in Ardour

Several months ago, i reported this bug with Guitarix plugins that sometimes causes Ardour (and no other hosts) to crash when closing the UI of a Guitarix plugin. Since i had mentioned having done some work on GxAmplifier, you jumped to the conclusion that my code was buggy, even after i mentioned it also happened with other guitarix plugins.

It would seem the problem is caused by having multiple instances of the same plugin, after changing some controls in one instance and on closing the UI of another one, boom. Plugins with more controls are more likely to be affected, which is why GxAmp (which i use heavily) was the first one to exhibit this bug.

Other users have been reporting this issue and similar ones mainly with Guitarix and Ardour. Verifying the LV2 specs with Hermann/Brummer, we confirmed that a callback is being fired from Ardour when it’s not supposed to, AFTER the UI is being torn down. My only guess as to why other plugins aren’t affected, is that the somewhat slow UI of Guitarix plugins exposes a race condition that normally goes unnoticed.

from Loading Guitarix plugins crashes Ardour · Issue #263 · brummer10/guitarix · GitHub –

This looks to me like the UI loop get called from the host after the plugin have receive the cleanup call. The plugin destroy all it's windows on that call.

    suil_x11_wrapper_idle (data=0x5555b148eb70) at ../libs/tk/suil/x11_in_gtk2.c:457
    https://github.com/Ardour/ardour/blob/master/libs/tk/suil/x11_in_gtk2.c#L457

 static gboolean
 suil_x11_wrapper_idle(void* data)
 {
 	SuilX11Wrapper* const wrap = SUIL_X11_WRAPPER(data);
 
 	wrap->idle_iface->idle(wrap->instance->handle);
 
 	return TRUE;  // Continue calling
 }

But LV2 specs says:

    /**
    Destroy the UI. The host must not try to access the widget after
    calling this function.
    */
    void (*cleanup)(LV2UI_Handle ui);

(about a possible way to fix)

You could try that. [...]
But, the point is, if that helps, it only indicate a bug in ardour. Because the specs say clearly,

    /**
    Destroy the UI. The host must not try to access the widget after
    calling this function.
    */
    void (*cleanup)(LV2UI_Handle ui);

and ui_idle() is running by the host, not by the plugin.

Hoping this gets some attention, because this bug is REALLY annoying. Only workaround now is to always remember to use the “generic” UI controls for guitarix plugins where i use more than one instance.

You should post the issue on the Bug Tracker
http://tracker.ardour.org/

Yep, will do, thanks.

You should post the issue on the Bug Tracker

Done.

Since this only affects guitarix plugins and no other LV2s it should be fixed there.

Host specific workaround for specific plugins are generally frowned upon.

Working around quirky behavior or bugs of specific plugins is one thing, but the argument here seems to be that for some reason these particular plugins exposed that Ardour is violating the LV2 specification.

except it isn’t. The instance is still present at this point.
It rather looks like guitarix is not following the specs.

To elaborate, Ardour does not interact with the widget after cleanup (as per spec), the extension data API is however still a valid entry point.

Since guitarix works in all other hosts then it’s a bug in ardour.

See, i like jumpinig to conclusions too.

1 Like

I’m not jumping to conclusions, I was involved with creating the LV2 standard, the Ardour’s LV2 implementation was done my Mr. LV2 himself…

I don’;t know the guitarix codebase, but I guess the fix would be trivial, just ignore idle callbacks or any other calls exposed by the extension interface when they are no longer appropriate.

1 Like

Fair enough, that is different than described,

All that being said, if there was a simple fix for this, I’d not be opposed to add it. But it is a bit of a chicken/egg issue. The widget (GTK_PLUG) being destroyed is what eventually leads to the idle call disconnect and cleanup call:

I also don’t see how the number of controls can have any effect here, and why multiple instances of the same plugin would be relevant.

YOU ARE CORRECT!! This really looks like a X11 resource ownership / lifetime bug. When a plugin is closed, ANOTHER guitarix plugin that has done any kind of text display through Cairo has a chance of causing the crash.

Either commenting out the call to XCloseDisplay () OR disabling all calls to cairo_show_text () in guitarix plugins completely eliminates the crash, but this either introduces a memory leak, or makes the plugins unusable. A “workable” hack is to use XDrawString () directly for displaying text, which makes it glitchy but usable - but that’s a temporary workaround rather than an actual fix.

It doesn’t directly, but it affects the amount of text being drawn. Quickest way to reproduce: load several guitarix plugins with COMBO BOXES that contain many items. (gxAmplifier, gxCabinet, etc) Open them. Use the mouse wheel to make the combo boxes draw text many times (scroll through the list, etc). Close one instance OTHER than the one(s) that did lots of text drawing. BOOM.

This is one bitch of a bug to track down, but it looks like we’re about to FINALLY nail it.

1 Like

Great detective work!

Yeah that sounds indeed like a nasty bug.

Why do plugins share resources to begin with? Can’t they all be self-contained, in which case this issue cannot happen?

Not entirely sure why a plugin like Guitarix would end up sharing resources. Maybe it wasn’t the standard back then.

What i’m wondering is, why each instance needs its own connection to the graphic system, and why it needs to be closed/reopened each time the UI is shown/hidden. If i load 5 instances of the same guitarix plugin, i see a separate call to XOpenDisplay () and XCloseDisplay () each time i open/close one of their UI… especially seeing that an X error (BadGlyph) crashes the entire host.

Because they are all independent and unrelated to each other. Plugins must not use shared resources by design. This has always been a fundamental part for Audio plugins on all platforms.

–
You could in theory do some kind of reference counting and/or use a mutex to clean up some static globals, but keep in mind that UI requests are asynchronous and different plugins may be updated concurrently which can lead to resource conflicts.

…so, if each plugin is a separate X client, how come an X error in one of them crashes all of Ardour?

Ardour (really libsuil) embedds the plugin’s XWindow. There is no process separation.

This issue is introduced by Ardour version > 8.4.0, in v8.4.0 guitarix plugins works as expected, v8.12.0 then introduce this Bug and the crash may happen. Ardour is the only host with this Problem.
I don’t know what exactly happen, but, to be clear, guitarix plugins didn’t share any resources with each other, nor use any global variables.
Guitarix plugins using cairo for text/rendering, which in turn use some shared resources (FreeFont, maybe Pango).

@delt explained it above:

if that was the issue a lot of other plugins would be affected by this as well…

Also in this case, would it not be the last instance that is being closed cause the issue (not the first)?

Do you have a backtrace of an actual crash, and a way to distinguish instances?