GNOME Bugzilla – Bug 784769
Deadlock in ges-launch between the gst_plugin_loading_mutex and class_init_rec_mutex
Last modified: 2017-07-11 14:40:41 UTC
This deadlock happens launching the following project: https://www.dropbox.com/s/2y0wip2ra2nyonw/deadlock_on_load_in_ges_launch.xges_tar?dl=0 Backtrace:
+ Trace 237627
Thread 5 (Thread 0x7fb3f0be5700 (LWP 20683))
Thread 1 (Thread 0x7fb3fb3d40c0 (LWP 20679))
$6 = 20679 Basically we are discovering a file which in turn leads to ffmpeg plugin loading (trying to get the class_init_rec_mutex while holding the in the Gst gst_plugin_loading_mutex) and at the same time we create a GES effect which in turns lead to trying to get the class_init_rec_mutex while holding gst_plugin_loading_mutex) -> Deadlock
So it's a locking order problem. Is there a way you could avoid gst_element_factory_make() from a _class_init function ? Or maybe we could drop the registry lock while loading an element, and retake it later (with obviously risking loading the same element twice, but when we take back the log, we could lookup the registry again maybe, we are already in the slow path anyway.
Second option is just not an option, since dropping that lock may lead to gst_plugin_register_func() being called twice for the same plugin in parallel. Some register function like gst_ffmpegaudenc_register() are just not thread safe from quick code inspection (would need a g_once or something to block, we just do g_type_from_name(), if (!type) ..., which may lead to two calls to g_type_register_static() (an undefined behaviour in glib according to the comment in the code).
Created attachment 355305 [details] [review] ges: Ref the GES class to avoid later deadlock This ensure that that all class are initialized from the main thread, avoid class initialization in random thread, which may cause deadlocks. This will effectively leak the class instance, as done in GStreamer, which mean the if GES is pulled from a shared object, it cannot be unloaded and reloaded anymore. This was probably already the case due to the dependency on GStreamer. This should fix this bug, as the GesEffect class init should have been run now at this point.
Review of attachment 355305 [details] [review]: I just tested, fixes the bug, though GESMetaContainer seems special: GLib-GObject-WARNING **: cannot retrieve class for invalid (unclassed) type 'GESMetaContainer'
(Side note, the provided project need manual path edit due to absolute path, am I using it wrong ?)
Created attachment 355307 [details] [review] [V2] ges: Ref the GES class to avoid later deadlock V2: MetaContainer is an interface, pulled by Asset already This ensure that that all class are initialized from the main thread, avoid class initialization in random thread, which may cause deadlocks. This will effectively leak the class instance, as done in GStreamer, which mean the if GES is pulled from a shared object, it cannot be unloaded and reloaded anymore. This was probably already the case due to the dependency on GStreamer.
Added unrefed and pushed as: commit c5eae31cf7cd30d2a42533bd9b60f89149261d2e (HEAD -> master, origin/master, origin/HEAD) Author: Nicolas Dufresne <nicolas.dufresne@collabora.com> Date: Mon Jul 10 21:42:21 2017 -0400 ges: Ref the GES class to avoid later deadlock This ensure that that all class are initialized from the main thread, avoid class initialization in random thread, which may cause deadlocks. https://bugzilla.gnome.org/show_bug.cgi?id=784769 Also backported to 1.12,(In reply to Nicolas Dufresne (stormer) from comment #5) > (Side note, the provided project need manual path edit due to absolute path, > am I using it wrong ?) You could have used `--ges-sample-path-recurse=file://path/to/extracted_files/` and GES would have handled it all for you.