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 361769 - Deadlock in gstpad.c
Deadlock in gstpad.c
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
0.10.6
Other Linux
: Normal normal
: 0.10.11
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2006-10-12 18:11 UTC by Yves Lefebvre
Modified: 2006-10-13 13:29 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
testcase (83.07 KB, application/x-compressed-tar)
2006-10-12 18:13 UTC, Yves Lefebvre
  Details
patch for this bug (731 bytes, patch)
2006-10-12 18:14 UTC, Yves Lefebvre
committed Details | Review

Description Yves Lefebvre 2006-10-12 18:11:31 UTC
Hi,

I found an hard to reproduce deadlock in gstreamer that occur when the pad are linking.

I also found a patch for it. The problem occur in gstreamer-0.10.6 and also in gstreamer-0.10.10

I'll start with a description of the problem:

first, we need to create an audio avi:
gst-launch-0.10 audiotestsrc num-buffers=10 ! avimux ! filesink location=testaudio.avi

and do this :
gst-launch-0.10 -f -v DUMMYsink name=sink location=out.raw filesrc location=testaudio.avi ! avidemux .audio_00 ! queue ! sink.audio_00

DUMMYsink is a sink filter (using collect data) that does nothing usefull (this is a test). The code is in attachment in folder dummysink. Also, I put in attachment a modified gstbin.c and gstpad.c that do sleep at some critical point to make the deadlock occurs faster (file from gstreamer-0.10.6). Those files are in folder sleep_bug/

When it deadlock, here's the stack trace :

(gdb) bt
  • #0 __kernel_vsyscall
  • #1 __lll_mutex_lock_wait
    from /lib/tls/i686/cmov/libpthread.so.0
  • #2 _L_mutex_lock_33
    from /lib/tls/i686/cmov/libpthread.so.0
  • #3 ??
  • #4 ??
    from /opt/trx/glib/2.8.6/lib/libglib-2.0.so.0
  • #5 ??
  • #6 __nanosleep_nocancel
    from /lib/tls/i686/cmov/libpthread.so.0
  • #7 ??
    from /opt/trx/gstreamer/gstreamer/0.10.6/lib/libgstreamer-0.10.so.0
  • #8 ??
  • #9 ??
  • #10 ??
  • #11 gst_pad_get_peer
    at gstpad.c line 2365
  • #12 gst_pad_get_peer
    at gstpad.c line 2365
  • #13 update_degree
    at gstbin.c line 1547
  • #14 IA__g_list_foreach
    at glist.c line 670
  • #15 gst_bin_sort_iterator_resync
    at gstbin.c line 1660
  • #16 gst_bin_sort_iterator_new
    at gstbin.c line 1700
  • #17 gst_bin_iterate_sorted
    at gstbin.c line 1731
  • #18 gst_bin_change_state_func
    at gstbin.c line 1805
  • #19 gst_pipeline_change_state
    at gstpipeline.c line 492
  • #20 gst_element_change_state
    at gstelement.c line 2177
  • #21 gst_element_set_state_func
    at gstelement.c line 2139
  • #22 gst_element_set_state
    at gstelement.c line 2049
  • #23 ??
  • #24 ??
  • #25 ??
  • #26 ??
  • #27 ??
  • #28 ??
  • #29 ??
    from /lib/tls/i686/cmov/libc.so.6
  • #30 ??
    from /lib/ld-linux.so.2
  • #31 ??
  • #32 ??
    from /lib/tls/i686/cmov/libpthread.so.0
  • #33 ??
  • #34 _dl_rtld_di_serinfo
    from /lib/ld-linux.so.2
  • #35 __libc_start_main
    from /lib/tls/i686/cmov/libc.so.6
  • #36 ??
  • #0 __kernel_vsyscall
  • #1 __lll_mutex_lock_wait
    from /lib/tls/i686/cmov/libpthread.so.0
  • #2 _L_mutex_lock_33
    from /lib/tls/i686/cmov/libpthread.so.0
  • #3 ??
  • #4 ??
  • #5 ??
  • #6 ??
  • #7 ??
  • #8 ??
    from /opt/trx/gstreamer/gstreamer/0.10.6/lib/libgstreamer-0.10.so.0
  • #9 ??
  • #10 ??
  • #11 ??
  • #12 gst_object_get_parent
    at gstobject.c line 835
  • #13 gst_object_get_parent
    at gstobject.c line 835
  • #14 gst_pad_link
    at gstpad.c line 1605
  • #15 pad_link_maybe_ghosting
    at gstutils.c line 1270
  • #16 gst_element_link_pads
    at gstutils.c line 1414
  • #17 gst_element_link_pads_filtered
    at gstutils.c line 1606
  • #18 gst_parse_found_pad
    at grammar.y line 410
  • #19 IA__g_cclosure_marshal_VOID__OBJECT
    at gmarshal.c line 636
  • #20 IA__g_closure_invoke
    at gclosure.c line 492
  • #21 signal_emit_unlocked_R
    at gsignal.c line 2485
  • #22 IA__g_signal_emit_valist
    at gsignal.c line 2244
  • #23 IA__g_signal_emit
    at gsignal.c line 2288
  • #24 gst_element_add_pad
    at gstelement.c line 636
  • #25 gst_avi_demux_parse_stream
    at gstavidemux.c line 1185
  • #26 gst_avi_demux_stream_header
    at gstavidemux.c line 2128
  • #27 gst_avi_demux_loop
    at gstavidemux.c line 2641
  • #28 gst_task_func
    at gsttask.c line 193
  • #29 g_thread_pool_thread_proxy
    at gthreadpool.c line 114
  • #30 g_thread_create_proxy
    at gthread.c line 564
  • #31 start_thread
    from /lib/tls/i686/cmov/libpthread.so.0
  • #32 clone
    from /lib/tls/i686/cmov/libc.so.6


I was able to find where it occur in the code :
In gstpad.c, inside function gst_pad_link_check_hierarchy when it call "gst_object_get_parent"
In gstbin.c, inside function update_degree,line 1544 when it call "gst_pad_get_peer"

what occurs is this 

Filter1-----Filter2

Thread 3 in gst_pad_link_prepare lock Filter 1 source pad and Filter 2 sink pad. later in gst_pad_link_check_hierarchy, it tries to get the grandparents of each pad using the gst_object_get_parent function. This function lock the element to do it's job.

Thread 1 lock Filter 2 element and try to do a pad_get_peer on it's sink pad. This function tries to lock the source pad of Filter 1.

The solution is not to use gst_object_get_parent inside the function gst_pad_link_check_hierarchy but GST_OBJECT_PARENT instead. The pointers are only used to evaluate if the condition fail so it seems safe to do so.

For example, using the files in folder sleep_fix/ work without deadlock.

I will post the attachment and patch next.
Comment 1 Yves Lefebvre 2006-10-12 18:13:13 UTC
Created attachment 74587 [details]
testcase

Testcase source for reproducing the deadlock
Comment 2 Yves Lefebvre 2006-10-12 18:14:56 UTC
Created attachment 74588 [details] [review]
patch for this bug
Comment 3 Wim Taymans 2006-10-13 11:44:05 UTC
indeed, well spotted. This is a case of where the locking order is not respected. Locking orde must be parent first, then child. 
Comment 4 Wim Taymans 2006-10-13 13:29:24 UTC
        * gst/gstpad.c: (gst_pad_link_check_hierarchy),
        (gst_pad_get_caps_unlocked), (gst_pad_save_thyself),
        (handle_pad_block), (gst_pad_push_event), (gst_pad_send_event):
        Patch by: Yves Lefebvre <ivanohe at abacom dot com> Fix possible deadlock
        due to wrong locking order. Fixes #361769.
        Remove some redundant/misplaced checks in pad_block.