It is not the instance on close which crash, but a remaining instance. You could close GxCabinet and GxAmplifier-x crash. The backtrace didn’t give us more then you’ll see in the guitarix issue page. It crash in XPending (). But the Dsiplay pointer is valid (I implemented a check for debugging) and it is not the ui_idle loop from the plugin which close. As I said, Ardour versions <= 8.4.0 didn’t have this issue. I don’t know if Ardour using now something like XESetFreeFont() or XESetCloseDisplay() under the hood.
The main difference is that since then system-wide gtk2 is no longer used, but it’s ancient history in software development, and telling users to use Arodur 8.3 for guitarix isn’t really an option.
No, no such calls in Ardour’s source tree (incl. x11/gtk) at all.
Ok, let’s try some debugging a-la Holmes:
Say you have one Guitarix plugin, and one LSP plugin, both use Pango/Cairo to draw text. Closing them works fine, right?
So now you load a second Guitarix plugin. What changes?
–
PS. I cannot reproduce this crash here. Neither with local build nor official Ardour 9 binary. Though I do have a 2024 (?) version of guitarix here from debian: guitarix-lv2 0.46.0+dfsg-1
It’s not easy to make it crash. You must move the combobox entry and the popdown menu and then close a other instance. On the UI side there have nothing changed in the LV2 plugs since the 0.46.0+dfsg-1 version from debian. I haven’t checked in combination with a LSP plugin, but, that’s a good idea. I’ll try it. Just, for now I’m running out of time, I must put that on my schedule for next week.
Or, maybe I should update the libxputty version from Guitarix to the last revision.
Here i’m consistently able to make it crash - load 4 or 5 instances of, let’s say GxAmplifier (because it has 3 combo boxes, and the amp model box has lots of entries), close one instance after scrolling through the combo boxes of the other instances.
So that might be the cause, or is there any other changes that might cause this bug?
Yep, so far was not able to make a guitarix plugin crash from another plugin, just tested with LSP Impulse Response scrolling several times through a directory with hundreds of IR files.
That’s what i was hoping the difference between Ardour 8.4 and 8.12 would point us towards. I hope to understand what exactly causes this bug, not just patch it / work around it.k
Honestly that’s the first thing i’d try.
Also a small update: i just noticed that if the instance is closed when scrolling through text boxes in other instance(s), THEN you open it, THEN you close it right after, the crash also happens.
Gonna do more testing once i get home…
No crash here after few minutes of test. 2 audio tracks each with 6 Guitarix-amp-x, Lsp-limiter and Lsp-Spectrum Analyzer.
openSUSE TW, Ardour-9.2 (official) and lv2-guitarix-0.47
Hard to say, but since libsuil’s x11_in_gtk is indirectly affected, it may be related.
Quick git log grep also shows that 8.2. → 8.3 fixes plugin window visibility when re-opening sessions (keep track of open windows, 3355e753b). Ardour 8.4 to 8.5 has some LV2 related realtime fixes (replace heap allocation with stack b96d55645, 6d29a0e99). 8.7 has optimization for port value changes and DSP/UI communication (7dac8994f69, 7dac8994f69) , LV2 thread priorities changed in 8.8 and the internal API was updated to C++11 (instead of boost)…
but nothing that can directly explain this.
What baffles me is that each gx *.lv2 plugin is a dedicated shared object:
$ ls -l /usr/lib/lv2/gx*.lv2/*gui*so | wc -l
69
So it’s odd that they can interfere with each other to begin with…
Okay, it looks like I’ve found a solution. First of the issue is deeply bounded in cairo X11 glyph handling. It seems that since Ardour > 8.4.0 the UI/Tread handling have changed and that leads to a race condition in XCloseDisplay() cleaning up Xft/Freetype XRender glyph while it is still in use by other plugs. The solution is simple, in guitarix plugs I replace all calls to cairo_show_text(cr, text) with
cairo_text_path(cr, text);
cairo_fill(cr);
that removes the dependency on Xft/Freetype and XRender glyph for this call, as it render the text as graphic, so don’t need glyph for it. With a quick test I can’t crash it any more. Still, give ma a week or so before I push it to the repository. Or, @delt , if you test that and found it working, send me a pull request, that may work faster.
@brummer Yesssss this seems to have fixed the bug completely!! I just hijacked
cairo_show_text (cairo_t *cr, const char *text)
to this simple wrapper function:
void cairo_show_text_dummy (cairo_t *cr, const char *text) {
cairo_text_path (cr, text);
cairo_fill (cr);
}
and the crash no longer happens after several minutes stress testing. All text showing correctly, with no (immediately) visible side effects.
Looks like we nailed it! Going to test further just to make sure…
1 Like
Will that have implications for clarity of text when screen or font scaling is in use?
I can’t see any issues here whatsoever, other than Guitarix plugins don’t get scaled along with other UI elements with preferences->appearance->font and scale …but that was also the case in previous/current version.
No. The only downside is that it is a little bit slower, as it now create the path, while before it use a cached version. But we talk about micro seconds here, it isn’t remarkable at all. So, from a users perspective there is no difference at all.
wow, it has been beautiful to follow along how you guys solve the problem all together.
So happy to experience this open-source community way of doing things.
2 Likes
The related issue on github:
and pull request that fixed this bug: