After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 607513 - input-selector segfaults in g_object_notify()
input-selector segfaults in g_object_notify()
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal blocker
: 0.10.32
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 624591 (view as bug list)
Depends on: 166020
Blocks: 614306
 
 
Reported: 2010-01-20 02:17 UTC by Kipp
Modified: 2010-12-31 11:29 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
example pipeline to demonstrate segfaults (might have to let it run for a while) (1.94 KB, text/x-python)
2010-01-20 02:17 UTC, Kipp
  Details
protect g_object_notify() calls with an existing mutex (2.08 KB, patch)
2010-01-20 02:22 UTC, Kipp
rejected Details | Review

Description Kipp 2010-01-20 02:17:32 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.
Comment 1 Kipp 2010-01-20 02:22:17 UTC
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.
Comment 2 Olivier Crête 2010-01-20 04:22:13 UTC
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.
Comment 3 Tim-Philipp Müller 2010-01-20 09:57:11 UTC
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.
Comment 4 Sebastian Dröge (slomo) 2010-01-24 19:54:44 UTC
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.
Comment 5 Sebastian Dröge (slomo) 2010-01-24 19:55:55 UTC
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.
Comment 6 Tim-Philipp Müller 2010-01-25 00:58:12 UTC
This locked up my totem on startup when trying to watch an .avi video. Will try to fix or get a stack trace tomorrow.
Comment 7 Tim-Philipp Müller 2010-01-25 10:18:15 UTC
Stack trace, playbin2 deadlock2 when playing ogg/vorbis file:

Thread 3 (Thread 0x7f9c4f895910 (LWP 6515))

  • #0 __lll_lock_wait
    at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S line 136
  • #1 _L_lock_949
    from /lib/libpthread.so.0
  • #2 __pthread_mutex_lock
    at pthread_mutex_lock.c line 61
  • #3 gst_input_selector_get_property
    at gstinputselector.c line 1035
  • #4 object_get_property
    at /build/buildd/glib2.0-2.23.1/gobject/gobject.c line 935
  • #5 IA__g_object_get_valist
    at /build/buildd/glib2.0-2.23.1/gobject/gobject.c line 1553
  • #6 IA__g_object_get
    at /build/buildd/glib2.0-2.23.1/gobject/gobject.c line 1643
  • #7 get_current_stream_number
    at gstplaybin2.c line 1454
  • #8 selector_active_pad_changed
    at gstplaybin2.c line 2298
  • #9 IA__g_closure_invoke
    at /build/buildd/glib2.0-2.23.1/gobject/gclosure.c line 767
  • #10 signal_emit_unlocked_R
    at /build/buildd/glib2.0-2.23.1/gobject/gsignal.c line 3247
  • #11 IA__g_signal_emit_valist
    at /build/buildd/glib2.0-2.23.1/gobject/gsignal.c line 2980
  • #12 IA__g_signal_emit
    at /build/buildd/glib2.0-2.23.1/gobject/gsignal.c line 3037
  • #13 g_object_dispatch_properties_changed
    at /build/buildd/glib2.0-2.23.1/gobject/gobject.c line 801
  • #14 gst_object_dispatch_properties_changed
    at gstobject.c line 509
  • #15 g_object_notify_queue_thaw
    at /build/buildd/glib2.0-2.23.1/gobject/gobjectnotifyqueue.c line 120
  • #16 IA__g_object_notify
    at /build/buildd/glib2.0-2.23.1/gobject/gobject.c line 888
  • #17 gst_selector_pad_event
    at gstinputselector.c line 362
  • #18 gst_pad_send_event
    at gstpad.c line 5042
  • #19 gst_pad_push_event
    at gstpad.c line 4898
  • #20 gst_pad_send_event
    at gstpad.c line 5042
  • #21 gst_pad_push_event
    at gstpad.c line 4898
  • #22 gst_pad_send_event
    at gstpad.c line 5042
  • #23 gst_pad_push_event
    at gstpad.c line 4898
  • #24 vorbis_handle_type_packet
    at gstvorbisdec.c line 704
  • #25 vorbis_handle_header_packet
    at gstvorbisdec.c line 753
  • #26 vorbis_dec_decode_buffer
    at gstvorbisdec.c line 982
  • #27 vorbis_dec_chain_forward
    at gstvorbisdec.c line 1163
  • #28 vorbis_dec_chain
    at gstvorbisdec.c line 1192
  • #29 gst_pad_chain_data_unchecked
    at gstpad.c line 4122
  • #30 gst_pad_push_data
    at gstpad.c line 4351
  • #31 gst_single_queue_push_one
    at gstmultiqueue.c line 919
  • #32 gst_multi_queue_loop
    at gstmultiqueue.c line 1101
  • #33 gst_task_func
    at gsttask.c line 238

Comment 8 Wim Taymans 2010-01-25 14:18:36 UTC
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.
Comment 9 Stefan Sauer (gstreamer, gtkdoc dev) 2010-08-24 07:58:26 UTC
*** Bug 624591 has been marked as a duplicate of this bug. ***
Comment 10 Sebastian Dröge (slomo) 2010-08-24 08:07:14 UTC
Ok, so we should introduce a new mutex here and it should be recursive?
Comment 11 Stefan Sauer (gstreamer, gtkdoc dev) 2010-08-24 14:32:36 UTC
I think so too. Kipp, would you have time to make a new patch?
Comment 12 Olivier Crête 2010-08-24 15:55:11 UTC
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.
Comment 13 Tim-Philipp Müller 2010-12-31 02:06:25 UTC
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.
Comment 14 Sebastian Dröge (slomo) 2010-12-31 11:22:52 UTC
I'll add a new static, recursive mutex for older GLib versions
Comment 15 Sebastian Dröge (slomo) 2010-12-31 11:28:56 UTC
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.