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 796935 - parsebin: Add missing locks/unlocks of the chain mutex
parsebin: Add missing locks/unlocks of the chain mutex
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
unspecified
Other All
: Normal normal
: 1.15.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-08-08 15:38 UTC by Sebastian Dröge (slomo)
Modified: 2018-10-28 17:05 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
parsebin: Add missing locks/unlocks of the chain mutex (1.85 KB, patch)
2018-08-08 15:38 UTC, Sebastian Dröge (slomo) 		committed Details | Review
parsebin.c (3.52 KB, text/x-csrc)
2018-08-09 08:51 UTC, Sebastian Dröge (slomo) 		  Details

Description Sebastian Dröge (slomo) 2018-08-08 15:38:08 UTC 
I don't know if this patch is correct or will introduce deadlocks elsewhere.
The same was solved in decodebin at some point and needs to be properly fixed
in parsebin too now in the new context.
Comment 1 Sebastian Dröge (slomo) 2018-08-08 15:38:14 UTC 
Created attachment 373288 [details] [review]
parsebin: Add missing locks/unlocks of the chain mutex

Before freeing pending pads it is required to hold the mutex, that's
what is protecting the list of pending pads in other places.
Comment 2 Sebastian Dröge (slomo) 2018-08-09 08:51:46 UTC 
Created attachment 373290 [details]
parsebin.c

Testcase, takes ms timeout as first argument and a filename/path as second. After the timeout the pipeline is shut down.

What value to take for the timeout depends on the machine, a couple of tens of ms. Without the patch there are all kinds of crashes and assertions happening.

With the patch there is sometimes still a problem with something erroring out because of "not-linked" after the parser but I'll debug that separately.
Comment 3 Sebastian Dröge (slomo) 2018-08-09 10:12:56 UTC 
The patch seems to cause deadlocks sometimes. Holding the chain mutex is a bad idea while adding pads or otherwise emitting signals or taking any stream lock.

+ Trace 238675

Thread 2 (Thread 0x7f64864b4700 (LWP 16251))

  • #0 __lll_lock_wait
    at ../sysdeps/unix/sysv/linux/x86_64/lowlevellock.S line 135
  • #1 __GI___pthread_mutex_lock
    at ../nptl/pthread_mutex_lock.c line 113
  • #2 post_activate
    at gstpad.c line 1049
  • #3 activate_mode_internal
    at gstpad.c line 1227
  • #4 gst_pad_activate_mode
    at gstpad.c line 1325
  • #5 gst_ghost_pad_activate_push_default
    at gstghostpad.c line 380
  • #6 activate_mode_internal
    at gstpad.c line 1220
  • #7 gst_pad_set_active
    at gstpad.c line 1118
  • #8 gst_parse_pad_set_blocked
    at gstparsebin.c line 4123
  • #9 gst_parse_pad_activate
    at gstparsebin.c line 4152
  • #10 expose_pad
    at gstparsebin.c line 2364
  • #11 gst_parse_chain_expose
    at gstparsebin.c line 3698
  • #12 gst_parse_bin_expose
    at gstparsebin.c line 3473
  • #13 pad_added_cb
    at gstparsebin.c line 2468
  • #14 caps_notify_cb
    at gstparsebin.c line 2576
  • #18 <emit signal notify:caps on instance 0x56254c669520 [GstPad]>
    at ../../../../gobject/gsignal.c line 3447
  • #19 g_object_dispatch_properties_changed
    at ../../../../gobject/gobject.c line 1082
  • #20 gst_object_dispatch_properties_changed
    at gstobject.c line 430
  • #21 g_object_notify_by_spec_internal
    at ../../../../gobject/gobject.c line 1175
  • #22 g_object_notify_by_pspec
    at ../../../../gobject/gobject.c line 1285
  • #23 store_sticky_event
    at gstpad.c line 5226
  • #24 gst_pad_push_event
    at gstpad.c line 5515
  • #25 gst_pad_set_caps
    at /home/slomo/Projects/gstreamer/head/gstreamer/gst/gstcompat.h line 59
  • #26 gst_tag_demux_set_src_caps
    at gsttagdemux.c line 372
  • #27 gst_tag_demux_element_find
    at gsttagdemux.c line 1388
  • #28 gst_tag_demux_element_loop
    at gsttagdemux.c line 1449
  • #29 gst_task_func
    at gsttask.c line 328
  • #30 g_thread_pool_thread_proxy
    at ../../../../glib/gthreadpool.c line 307
  • #31 g_thread_proxy
    at ../../../../glib/gthread.c line 784
  • #32 start_thread
    at pthread_create.c line 463
  • #33 clone
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S line 95 






Comment 4


Sebastian Dröge (slomo)





          2018-10-28 17:04:48 UTC
        



Attachment 373288 [details] pushed as 596a4ee - parsebin: Add missing locks/unlocks of the chain mutex