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 631853 - [queue2] deadlock when using temp-location and dispatch-properties
[queue2] deadlock when using temp-location and dispatch-properties
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
unspecified
Other Linux
: Normal major
: 0.10.31
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-10-11 08:00 UTC by Edward Hervey
Modified: 2010-10-11 16:14 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Edward Hervey 2010-10-11 08:00:18 UTC
(gdb) bt
  • #0 __lll_lock_wait
    at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S line 136
  • #1 _L_lock_945
    from /lib/libpthread.so.0
  • #2 __pthread_mutex_lock
    at pthread_mutex_lock.c line 61
  • #3 gst_queue2_get_property
    at gstqueue2.c line 2940
  • #4 object_get_property
    at gobject.c line 935
  • #5 IA__g_object_get_valist
    at gobject.c line 1555
  • #6 IA__g_object_get
    at gobject.c line 1645
  • #7 IA__g_closure_invoke
    at gclosure.c line 767
  • #8 signal_emit_unlocked_R
    at gsignal.c line 3248
  • #9 IA__g_signal_emit_valist
    at gsignal.c line 2981
  • #10 IA__g_signal_emit
    at gsignal.c line 3038
  • #11 gst_object_dispatch_properties_changed
    at gstobject.c line 543
  • #12 g_object_notify_queue_thaw
    at gobjectnotifyqueue.c line 120
  • #13 IA__g_object_notify
    at gobject.c line 888
  • #14 gst_queue2_open_temp_location_file
    at gstqueue2.c line 1353
  • #15 gst_queue2_change_state
    at gstqueue2.c line 2774
  • #16 gst_element_change_state
    at gstelement.c line 2603
  • #17 gst_element_set_state_func
    at gstelement.c line 2559
  • #18 gst_bin_element_set_state
    at gstbin.c line 2171
  • #19 gst_bin_change_state_func
    at gstbin.c line 2470
  • #20 gst_uri_decode_bin_change_state
    at gsturidecodebin.c line 2233
  • #21 gst_element_change_state
    at gstelement.c line 2603
  • #22 gst_element_set_state_func
    at gstelement.c line 2559
  • #23 gst_bin_element_set_state
    at gstbin.c line 2171
  • #24 gst_bin_change_state_func
    at gstbin.c line 2470
  • #25 gst_pipeline_change_state
    at gstpipeline.c line 475
  • #26 gst_play_bin_change_state
    at gstplaybin2.c line 3475
  • #27 gst_element_change_state
    at gstelement.c line 2603
  • #28 gst_element_set_state_func
    at gstelement.c line 2559
  • #29 bacon_video_widget_open
    at bacon-video-widget-gst-0.10.c line 3745

Comment 1 Edward Hervey 2010-10-11 08:03:43 UTC
To reproduce this, open totem, open a network location, let it play a bit, then open another network location.

The problem is because when gst_queue2_open_temp_location_file() is called from the change_state method, the queue2_mutex_lock is taken...

and _open_temp_location_file emits g_object_notify() which, when dispatch properties is enabled, will try to get the value of that property, going through queue2's _get_property() which grabs the same mutex

=> deadlock
Comment 2 Sebastian Dröge (slomo) 2010-10-11 08:30:00 UTC
Calling g_object_notify() with the same lock that is used to protect get/set_property is an interesting idea :)

In gst_queue2_src_activate_pull() the _open_temp_location_file() is called without the queue2 mutex, maybe it's ok if the mutex is taken after opening the temp file? If not, gst_queue2_src_activate_pull() has to be changed too. If the mutex is really necessary here (I don't think so, all accesses to the temp file should be protected by other mutexes already) open_temp_location() could return some boolean in an out parameter to have g_object_notify() called outside the function.
Comment 3 Wim Taymans 2010-10-11 16:14:22 UTC
commit 62ffd66f109c5c17fa8b49a69a9a382ca5c76541
Author: Wim Taymans <wim.taymans@collabora.co.uk>
Date:   Mon Oct 11 18:10:07 2010 +0200

    queue2: release queue2 lock before notify
    
    Make sure that we don't hold the lock when we notify the temp-location
    property,
    
    Fixes #631853