GNOME Bugzilla – Bug 607513
input-selector segfaults in g_object_notify()
Last modified: 2010-12-31 11:29:04 UTC
Created attachment 151803 [details] example pipeline to demonstrate segfaults (might have to let it run for a while) input-selector can call g_object_notify() from multiple threads. apparently this function is not thread-safe (!?) and the result is a segfaults in the input-selector element. I have not studied the problem carefully so this might be a bad solution from a bigger-picture design perspective but within input-selector, with only one exception, g_object_notify() is always called immediately after GST_INPUT_SELECTOR_UNLOCK() and by moving the mutex release to after the call to g_object_notify() the segfaults are eliminated.
Created attachment 151804 [details] [review] protect g_object_notify() calls with an existing mutex I haven't studied the problem carefully, so I don't know what happens if a handler for the notify signal tries to invoke a method on the input-selector instance given the mutex being held with g_object_notify() is called. Hopefully it doesn't lock up. I'm sure the non-thread-safety of g_object_notify() is a big deal, so maybe there's a proper "standard" solution to this like some kind of global glib mutex or something. Anyway, this patch stops the example script stop segfaulting.
Having the mutex in there is probably not enough, since g_object_notify() is also called by g_object_set(). This is actually bug #166020.
True, it's not enough, but it should reduce the odds of this bug occuring considerably, especially in a context like this where you get input from multiple threads. We've protected the explicit notifies with locks elsewhere too, e.g. in fakesink. Ultimately it needs fixing in GLib of course. If we patch this in -bad, we should also patch the playbin2-internal copy of input selector in -base.
commit 7e067615ffb5614f068f7753c10dde99afe49c3c Author: Kipp Cannon <kcannon@ligo.caltech.edu> Date: Sun Jan 24 20:53:00 2010 +0100 inputselector: Protect g_object_notify() with the object's mutex This works around the thread unsafety of g_object_notify() Fixes bug #607513.
commit a37426c41c80fd21e5017fea01a786c05bcd9661 Author: Kipp Cannon <kcannon@ligo.caltech.edu> Date: Sun Jan 24 20:55:26 2010 +0100 inputselector: Protect g_object_notify() with the object's mutex This works around the thread unsafety of g_object_notify() Fixes bug #607513.
This locked up my totem on startup when trying to watch an .avi video. Will try to fix or get a stack trace tomorrow.
Stack trace, playbin2 deadlock2 when playing ogg/vorbis file:
+ Trace 220247
Thread 3 (Thread 0x7f9c4f895910 (LWP 6515))
reverted for the release. commit fcf2668b208af31af584a161073019ca4ecf087b Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Mon Jan 25 12:22:17 2010 +0100 Revert "inputselector: Protect g_object_notify() with the object's mutex" This reverts commit a37426c41c80fd21e5017fea01a786c05bcd9661, it's causing deadlocks with playbin2.
*** Bug 624591 has been marked as a duplicate of this bug. ***
Ok, so we should introduce a new mutex here and it should be recursive?
I think so too. Kipp, would you have time to make a new patch?
g_object_notify has been made thread safe in the GLib head.. I guess the easy solution for now is to just not use it (and instead use a regular signal). And switch back to GObject Notify when we can depend on the fixed GLib release.
Could just do a warning or something somewhere when GLib < 2.26 is used and mention it in release notes. Maybe skip emitting the tags signal or serialise through static global mutex or something to minimise breakage.
I'll add a new static, recursive mutex for older GLib versions
commit 3f89954c271e985a6d20d5cf1266cd2d448753df Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Fri Dec 31 12:27:45 2010 +0100 inputselector: Protected g_object_notify() calls for the active-pad with a r This works around a thread safety problem in GLib < 2.26.0 and should be removed when we depend on 2.26.0. Fixes bug #607513.