Fluida loses GUI state on hide/show window; segfaults on second add

[sebageek]

Let me start with this disclaimer: I'm not sure if this is a problem with my setup, with Fluida or with reaper. I can observe something similar when I'm using carla though.

I'm trying to use Fluida with reaper 6.71 + FluidR3_GM.sf2. I started out using a self compiled version of current master, but then went on to the prebuild version linked in the midi for a common baseline (most notably I compiled against libfluidsynth3, prebuild uses 2). I can add the plugin as a FX to a track, play the piano, hear sound, change instruments.

The moment I hide the gui (closing and reopening the FX window or changing to another plugin, then back to Fluida) Fluida loses its GUI state. It shows None as current path and current selected input, all knobs/buttons are back at starting position. I can still hear the preselected instrument. If I had the reverb turned on previously, I could now turn it on and then off again to get it off, but the old state is somewhere stuck in there. This is reproducible with my selfcompiled and the stock version of Fluida on my system.

Secondly, if I now remove the plugin and readd it to my FX chain, then reaper segfaults. This is the stacktrace I got from coredumpctl:

Stack trace of thread 24837:
#0  0x00007fafd77744e7 __strcmp_avx2 (libc.so.6 + 0x1554e7)
#1  0x00007fafd6798c09 g_str_equal (libglib-2.0.so.0 + 0x42c09)
#2  0x00007fafd679770a g_hash_table_lookup (libglib-2.0.so.0 + 0x4170a)
#3  0x00007fafd67babf4 g_intern_static_string (libglib-2.0.so.0 + 0x64bf4)
#4  0x00007fafc1567f8d n/a (libinstpatch-1.0.so.2 + 0x29f8d)
#5  0x00007fafc15682e5 ipatch_base_get_type (libinstpatch-1.0.so.2 + 0x2a2e5)
#6  0x00007fafc15c30a2 ipatch_init (libinstpatch-1.0.so.2 + 0x850a2)
#7  0x00007fafd53608b0 new_fluid_synth (libfluidsynth.so.2 + 0x378b0)
#8  0x00007fafd59b4e60 n/a (/home/seba/.lv2/Fluida.lv2/Fluida.so + 0xbe60)
ELF object binary architecture: AMD x86-64

Any idea what could be causing this?

[brummer10]

Hi
Thanks for reporting the issues. I could reproduce them here.
I could fix the loses GUI state, but can't fix the segfault on second load.
This issue is specific to Reaper. I noticed the same issue in Reaper for all plugs using fluidsynth. There is no issue to load multiple instances of Fluida in Reaper, and, as long one instance is active, you could load/unload multiple instances without issues. Only when you remove the last active instance you can't load a new one any more without reaper crash.
This issue exist within no other LV2 host.

[brummer10]

This is somewhat related to the crash: swami/libinstpatch#63 (comment)

[sebageek]

Thanks for the quick response!

I've tried out the patch. It looks like most values are restored, except for:

• current instrument - this is always reset to 000 000 Piano 1 aka the first instrument in the font
• depth - this is always reset to 0

[sebageek]

I've continued investigating the crash a bit. I can reproduce it on reaper, bespoke and carla. I can also reproduce this with other libfluidsynth based (e.g. https://github.com/falkTX/FluidPlug/tree/master/FluidGM.lv2). The problem here seems to be the registering/reinitialization of libinstpatch by fluidsynth. The first run of running libinstpatch's ipatch_init() registers a lot of glib stuff (e.g. GTypes), which hang around even after library deinitialization. Apparently there is also no way to remove these objects in glib. On Next initialization of libinstpatch these types are re-registered. For some this works, for others apparently not - but that's where I stopped to far. In my gdb session I hang at g_type_class_ref(IPATCH_TYPE_DLS2);.

But anyway, this is not a Fluida problem.

[brummer10]

Yes, that's what I found as well. I've no idea how that could be fixed, unfortunately.
I remember that in early LV2 there was a option were plugs could inform a host to not release a library when the plug gets removed. That would be the only way to fix this issue,
What I could fix is the UI issue, that should work now as expected.

[sebageek]

Depth seems to be fixed, but for the instrument I ow see the following behavior:

• set instrument to something else than first instrument
• first close/reopen of gui: instrument is restored correctly in gui, but switches back to first instrument in sound/DAW (I hear the piano instead of e.g. the musicbox)
• second close/reopen (if I don't touch the instrument): instrument is set to the first instrument in soundfont again (sound is still at piano)

Interesting idea with leaving the library open beyond the plugin's lifetime, would probably solve this. But if that's not an option... I'd probably try my luck with libinstpatch and see if we can find a way around re-registering resources - though my glib expertise is severely limited ;)

[brummer10]

Ah, yes. Was a bit to lazy by check the fix. Now it should really be fixed.

[brummer10]

The issue with libinstpatch is for hosts easily solvable. All they need to do is including <libinstpatch/libinstpatch.h>
and run ipatch_init(); link with -linstpatch-1.0
that will keep the instance counter of libinstpatch up so that it didn't try to re-init itself.
As. if a plugin do that it only counts for the lifetime of the plugin.

[brummer10]

Turns out that there is a solution.
When I add in fluida.cpp
#include <dlfcn.h>

and in line 867 (in Fluida_::instantiate(. . . )
dlopen("libinstpatch-1.0.so", RTLD_LAZY);
libinstpatch becomes instantiated even after the plug close. This avoid this issue completely.
Unfortunately then there is no way to call dlclose.
A quick check shows that libinstpatch close then on application (Reaper) exit.
cat /proc/*/maps | awk '{print $6;}' | grep '\.so' | sort | uniq | grep libinstpatch

[sebageek]

Solution for GUI state works now on my machine, thanks for the fix!

dlopening libinstpatch and then not closing it on unload would leave the libary handle dangling, which would on the one hand solve the problem, but on the other hand would get Fluida to leak memory / sockets (I think), which could be inconvenient when Fluida is used in larger projects, where it is loaded many times. I think for me I'd rather go with "keep one instance of any plugin using libinstpatch open" than leaking stuff from the plugin. It also seems to work if at least any plugin is still open, it doesn't need to be the first instance.

[brummer10]

I'm not sure, but it feels wrong to add a library into the host processing space this way. So it will be bad habit at least I guess.
Anyway I've done a quick check with valgrind to check the leaking, this is the result for fluida using dlopen in reaper

==152356== LEAK SUMMARY:
==152356==    definitely lost: 122,226 bytes in 1,427 blocks
==152356==    indirectly lost: 16,116,781 bytes in 7,920 blocks
==152356==      possibly lost: 4,177,074 bytes in 2,900 blocks
==152356==    still reachable: 7,976,827 bytes in 20,305 blocks
==152356==         suppressed: 0 bytes in 0 blocks

and this is with fluida don't using dlopen

==152874== LEAK SUMMARY:
==152874==    definitely lost: 122,234 bytes in 1,429 blocks
==152874==    indirectly lost: 16,015,548 bytes in 7,884 blocks
==152874==      possibly lost: 4,277,336 bytes in 2,936 blocks
==152874==    still reachable: 7,976,706 bytes in 20,305 blocks
==152874==         suppressed: 0 bytes in 0 blocks

using multiple calls to dlopen from the same process (so, multiple instances of fluida) doesn't change that. So it seems leaking isn't really a problem with this approve.

[sebageek]

I definitely agree on the bad habit part.

Interesting that it doesn't leak any memory. I also thought it would at least leave an extra handle open, but apparently dlopen() is smart enough to only load the library once. If the library gets closed and reopened, it's at a different memory address, but if someone opens it and leaves if open, all subsequent calls get the same library address, even when they close it. It feels almost refcounted. But as this is all speculation based on tests I did by writing some .so code I actually bothered to spend the two minutes to read the manpage of dlopen() and let me quote:

       If the same shared object is opened again with dlopen(), the same object  han‐
       dle  is  returned.   The  dynamic linker maintains reference counts for object
       handles, so a dynamically loaded shared object is not  deallocated  until  dl‐
       close()  has  been called on it as many times as dlopen() has succeeded on it.
       Constructors (see below) are called only when the object  is  actually  loaded
       into memory (i.e., when the reference count increases to 1).

So I guess this is a way you could solve this in Fluida. As long as Fluida is the first plugin using something libinstpatch-based, it should work for/with all other processes, only downsite being the open .so handle dangling (the fd will remain until the host program terminates). If you'd start with something else, like FluidGM, Fluida will break, though. So... yeah, workaround for Fluida. I think I'll open a bug with libinstpatch regardless.

[terminator356]

Hi! We're observing these crashes in MusE as well, after a report.
I have followed the threads and I think I understand the problem.

I just want to relate how MusE does things, it seems related to this discussion.

MusE has a built-in synth plugin architecture called MESS. We have several synth plugins.
They each are dynamic libraries, like any other plugin architecture, and they are supposed
to be completely independent, with the goal that one day another host might want to load them.

One of our MESS plugins is our famous fluidsynth plugin.
It uses fluidsynth (of course), and libinstpatch strictly to get all the drum note names.

With this discussion, I was curious why we don't get any crashes with our fluidsynth plugin.

A quick review reminded me that I seem to be cheating.
I tell our main app's cmake to use fluidsynth and libinstpatch, and then I simply tell
our fluidsynth plugin's cmake to link with those libraries.
(I am also linking with Qt libraries, loaded by the main app, for the graphics. Seems another cheat there.)

So it seems that our fluidsynth plugin would not be quite independent after all.
It seems another host would have to load fluidsynth and libinstpatch (and Qt)
so that our plugin could then link with them.

Does that seem to make sense? That is, why it does work OK for us.

Cheers.
Tim.

[brummer10]

Hi
As pointed out earlier in this thread, the issue is easily solvable by hosts for all plugins.
Just including <libinstpatch/libinstpatch.h>
and run ipatch_init(); in your constructor.
that will keep the instance counter of libinstpatch up for the lifetime of the host, and not only for the lifetime of the plugin. So then, when a plugin get loaded which use it, it get a instance pointer from libinstpatch and increase the reference counter by 1. When it get unloaded, libinstpatch will still be instantiated by the reference counter of the host. Then when load the plug again, it gets again a instance pointer instead try to load the symbol table into glib.

libisntpatch will be unloaded then automatically when you run your destructor, no extra call to deinit needed.

regards
hermann

[terminator356]

Done!
No more crashes here in MusE so far.
Thanks very much for your help.

[brummer10]

Great!

Regardless of my previous comment (No need to deinit), I think it wouldn't hurt to call
ipatch_close();
in the destructor, for clear clean code.

regards
hermann

[terminator356]

Checked: We never did call ipatch_close() anywhere.
I have now added it.

To summarize, we now call ipatch_init() in our fluidsynth MESS plugin constructor and our main.cpp,
and we call ipatch_close() in our fluidsynth MESS plugin destructor and at the end of our main.cpp.

That way the main app ultimately owns the library.
Hopefully the reference counting will take care of the rest, and unload the library when appropriate.