swami / libinstpatch

Instrument file software library.
Other
20 stars 6 forks source link

libinstpatch segfaults/hangs when initialized two times via shared objects #70

Open sebageek opened 1 year ago

sebageek commented 1 year ago

I use libinstpatch through various lv2 plugins that are based on fluidsynth. When I load a plugin, unload it and reload it in a plugin host then the last load hangs indefinitely or segfaults inside instpatch_init(). This only happens if the plugins are loaded as dynamic shared objects. I can reproduce this with Fluida.lv2 or FluidGM in reaper, carla or BespokeSynth.

So, what's problematic here is the second load of a plugin using libinstpatch. The first one works fine, removal works fine, but adding it (or a different plugin using libinstpatch) again crashes or hangs the plugin host.

To reproduce this in a smaller setup I have written a small programm with a .so that calls instpatch_init() and then instpatch_close(). The program can be found here in a gist: https://gist.github.com/sebageek/8c9aedc123f38347a0307827770e397f

Here is the output:

$ make run
./borkme
Before load (handle is 0x564eb8651ed0)
Running bork instpatch
After load
Before load (handle is 0x564eb8652490)
Running bork instpatch

(process:20637): GLib-GObject-WARNING **: 21:45:49.501: cannot register existing type 'IpatchSplitsType'

(process:20637): GLib-GObject-CRITICAL **: 21:45:49.501: g_param_spec_enum: assertion 'G_TYPE_IS_ENUM (enum_type)' failed

** (process:20637): CRITICAL **: 21:45:49.501: ipatch_type_install_property: assertion 'G_IS_PARAM_SPEC(prop_spec)' failed

(process:20637): GLib-GObject-CRITICAL **: 21:45:49.501: g_boxed_type_register_static: assertion 'g_type_from_name (name) == 0' failed

(process:20637): GLib-GObject-WARNING **: 21:45:49.502: cannot register existing type 'IpatchSF2GenType'

(process:20637): GLib-GObject-WARNING **: 21:45:49.502: cannot retrieve class for invalid (unclassed) type '<invalid>'

** (process:20637): CRITICAL **: 21:45:49.502: file ./libinstpatch/IpatchSF2Gen.c: line 152 (_ipatch_sf2_gen_init): assertion `enum_class != NULL' failed.

(process:20637): GLib-GObject-CRITICAL **: 21:45:49.502: g_type_class_unref: assertion 'g_class != NULL' failed

(process:20637): GLib-GObject-WARNING **: 21:45:49.502: cannot register existing type 'IpatchSample'

(process:20637): GLib-GObject-WARNING **: 21:45:49.502: cannot register existing type 'IpatchItem'

(process:20637): GLib-GObject-CRITICAL **: 21:45:49.502: g_type_interface_add_prerequisite: assertion 'G_TYPE_IS_INTERFACE (interface_type)' failed

(process:20637): GLib-GObject-WARNING **: 21:45:49.502: cannot register existing type 'IpatchSF2GenItem'

(process:20637): GLib-GObject-WARNING **: 21:45:49.502: cannot register existing type 'IpatchItem'

(process:20637): GLib-GObject-CRITICAL **: 21:45:49.502: g_type_interface_add_prerequisite: assertion 'G_TYPE_IS_INTERFACE (interface_type)' failed

(process:20637): GLib-GObject-WARNING **: 21:45:49.502: cannot register existing type 'IpatchSF2ModItem'

(process:20637): GLib-GObject-WARNING **: 21:45:49.502: cannot register existing type 'IpatchItem'

(process:20637): GLib-GObject-CRITICAL **: 21:45:49.502: g_type_interface_add_prerequisite: assertion 'G_TYPE_IS_INTERFACE (interface_type)' failed

(process:20637): GLib-GObject-WARNING **: 21:45:49.502: cannot register existing type 'IpatchSF2VoiceCache'

(process:20637): GLib-GObject-WARNING **: 21:45:49.502: cannot retrieve class for invalid (unclassed) type '<invalid>'

(process:20637): GLib-GObject-WARNING **: 21:45:49.502: cannot register existing type 'IpatchItem'

(process:20637): GLib-GObject-CRITICAL **: 21:45:49.502: g_type_register_static: assertion 'parent_type > 0' failed

(process:20637): GLib-GObject-CRITICAL **: 21:45:49.502: g_type_register_static: assertion 'parent_type > 0' failed

(process:20637): GLib-CRITICAL **: 21:45:49.502: g_once_init_leave: assertion 'result != 0' failed

(process:20637): GLib-GObject-WARNING **: 21:45:49.502: cannot retrieve class for invalid (unclassed) type '<invalid>'

(process:20637): GLib-GObject-WARNING **: 21:45:49.502: cannot register existing type 'IpatchItem'

(process:20637): GLib-GObject-CRITICAL **: 21:45:49.502: g_type_register_static: assertion 'parent_type > 0' failed

(process:20637): GLib-GObject-WARNING **: 21:45:49.503: cannot retrieve class for invalid (unclassed) type '<invalid>'

(process:20637): GLib-GObject-WARNING **: 21:45:49.503: cannot register existing type 'IpatchConverter'

(process:20637): GLib-GObject-WARNING **: 21:45:49.503: cannot retrieve class for invalid (unclassed) type '<invalid>'

Running this in gdb gives me this backtrace:

^C
Program received signal SIGINT, Interrupt.
syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
38  ../sysdeps/unix/sysv/linux/x86_64/syscall.S: No such file or directory.
(gdb) bt
#0  syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
#1  0x00007ffff786f7af in g_cond_wait (cond=cond@entry=0x7ffff78fcfe0 <g_once_cond>, mutex=mutex@entry=0x7ffff78fcff0 <g_once_mutex>)
    at ../../../glib/gthread-posix.c:1590
#2  0x00007ffff7845e9b in g_once_init_enter (location=location@entry=0x7ffff7a1d6e0 <g_define_type_id.volatile>)
    at ../../../glib/gthread.c:715
#3  0x00007ffff79872dc in ipatch_base_get_type () at ./libinstpatch/IpatchBase.c:71
#4  0x00007ffff798ef89 in ipatch_dls2_get_type () at ./libinstpatch/IpatchDLS2.c:96
#5  0x00007ffff79e20c9 in ipatch_init () at ./libinstpatch/misc.c:261
#6  0x00007ffff7fbf141 in bork_instpatch () at instbork.c:6
#7  0x0000555555555293 in load_and_call(char const*, char const*)
    (libname=0x555555556042 "./instbork.so", funname=0x555555556033 "bork_instpatch") at borkme.cpp:14
#8  0x0000555555555307 in main() () at borkme.cpp:21

So to me it looks like the registration of all the custom GTypes results in this behavior, as when (and this is where my guesswork begins) libinstpatch is mapped to a new address the second time the reregistration of all types somehow causes problems. According to the glib documentation there is no way to unregister resources, so I'm not sure how to solve this.

In this issue of Fluida (ff.) it was discussed that leaving a handle open to libinstpatch would get the OS to not remap libinstpatch to a new memory address, thus keep it working, but this is rather a dirty hack than a proper soltion.

The other option - I think - could be to check if the types are already registered, and if so, use them? I tried my hand at this with this small patch, but it only helps for where the GType helpers are written out and not generated by a macro:

diff --git a/libinstpatch/IpatchDLS2.c b/libinstpatch/IpatchDLS2.c
index b34e470..2632e83 100644
--- a/libinstpatch/IpatchDLS2.c
+++ b/libinstpatch/IpatchDLS2.c
@@ -84,17 +84,22 @@ ipatch_dls2_get_type(void)

     if(!item_type)
     {
-        static const GTypeInfo item_info =
+        // check if item is already registered
+        item_type = g_type_from_name("IpatchDLS2");
+        if(!item_type)
         {
-            sizeof(IpatchDLS2Class), NULL, NULL,
-            (GClassInitFunc)ipatch_dls2_class_init, NULL, NULL,
-            sizeof(IpatchDLS2),
-            0,
-            (GInstanceInitFunc)ipatch_dls2_init,
-        };
-
-        item_type = g_type_register_static(IPATCH_TYPE_BASE, "IpatchDLS2",
-                                           &item_info, 0);
+            static const GTypeInfo item_info =
+            {
+                sizeof(IpatchDLS2Class), NULL, NULL,
+                (GClassInitFunc)ipatch_dls2_class_init, NULL, NULL,
+                sizeof(IpatchDLS2),
+                0,
+                (GInstanceInitFunc)ipatch_dls2_init,
+            };
+
+            item_type = g_type_register_static(IPATCH_TYPE_BASE, "IpatchDLS2",
+                                               &item_info, 0);
+        }
     }

     return (item_type);

As I have not replaced all the macros (e.g. _DEFINE_TYPE(IpatchFile, ipatch_file, IPATCH_TYPE_ITEM)) I'm not even sure if this will work (are there pointers to now invalid memory of registered resources in glib?).

I've tested this with libinstpatch-1.0-2 (1.1.6-1) from Debian testing and with compiling libinstpatch from current master. Any idea how to solve this so dynamically loaded plugins using libinstpatch won't crash/hang their plugin host?

derselbst commented 1 year ago

IIRC, gobject does not allow (or support) shared libraries to be loaded and unloaded as you like. Your observations about the GType stuff seem to be familiar to me. Pls. try again by removing the calls to dlopen and dlclose - I would expect it to work.

Apart form that I must admit, that I have little knowledge about gobject and even less time to look into this. If you come up with a solution / workaround I would thankfully integrate it.