Bug 785987 – adaptivedemux: Deadlock on stopping pipeline

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 785987 - adaptivedemux: Deadlock on stopping pipeline


Summary:	adaptivedemux: Deadlock on stopping pipeline


Status:	RESOLVED FIXED

Product:	GStreamer
Classification:	Platform
Component:	gst-plugins-bad
Version:	1.12.2
Hardware:	Other Linux

Importance:	Normal normal
Target Milestone:	1.12.3
Assigned To:	GStreamer Maintainers
QA Contact:	GStreamer Maintainers

URL:
Whiteboard:

Depends on:
Blocks:

Reported:	2017-08-08 08:47 UTC by davecraig
Modified:	2017-08-11 08:07 UTC

See Also:
GNOME target:	---
GNOME version:	---

Attachments
Possible patch (3.52 KB, patch) 2017-08-08 08:47 UTC, davecraig	none	Details \| Review
Patch in git format (4.15 KB, patch) 2017-08-08 09:20 UTC, davecraig	none	Details \| Review
Proposed patch (4.33 KB, patch) 2017-08-08 09:24 UTC, davecraig	committed	Details \| Review

Description davecraig 2017-08-08 08:47:27 UTC

Created attachment 357171 [details] [review]
Possible patch

If a pipeline is stopped when there are pending_streams in an adaptiveplaylist, then there can be a deadlock. gst_adaptive_demux_stop_tasks only stops demux->streams and not demux->pending_streams which can also be running. 

To test this I was playing 
http://hls.iptv.optimum.net/news12/nipadlive/index_new.m3u8?callsign=N12KN

and continuously restarting playback after a random delay of 0 to 10 seconds. It would deadlock within a minute or two. With the attached patch there is no longer a deadlock.

+ Trace 237785

Thread 21 (LWP 1501)

#0 syscall
at ../sysdeps/unix/sysv/linux/arm/syscall.S line 37
#1 g_cond_wait
at ../../glib-2.50.2/glib/gthread-posix.c line 1395
#2 gst_adaptive_demux_stream_push_buffer
at ../../../../clone/gst-libs/gst/adaptivedemux/gstadaptivedemux.c line 2308
#3 gst_hls_demux_handle_buffer
at ../../../clone/ext/hls/gsthlsdemux.c line 927
#4 gst_hls_demux_data_received
at ../../../clone/ext/hls/gsthlsdemux.c line 1040
#5 _src_chain
at ../../../../clone/gst-libs/gst/adaptivedemux/gstadaptivedemux.c line 2524
#6 gst_pad_chain_data_unchecked
at ../../clone/gst/gstpad.c line 4220
#7 gst_pad_push_data
at ../../clone/gst/gstpad.c line 4464
#8 gst_pad_push
at ../../clone/gst/gstpad.c line 4583
#9 gst_proxy_pad_chain_default
at ../../clone/gst/gstghostpad.c line 127
#10 gst_pad_chain_data_unchecked
at ../../clone/gst/gstpad.c line 4220
#11 gst_pad_push_data
at ../../clone/gst/gstpad.c line 4464
#12 gst_pad_push
at ../../clone/gst/gstpad.c line 4583
#13 gst_queue_push_one
at ../../../clone/plugins/elements/gstqueue.c line 1365
#14 gst_queue_loop
at ../../../clone/plugins/elements/gstqueue.c line 1517
#15 gst_task_func
at ../../clone/gst/gsttask.c line 335
#16 g_thread_pool_thread_proxy
at ../../glib-2.50.2/glib/gthreadpool.c line 307
#17 g_thread_proxy
at ../../glib-2.50.2/glib/gthread.c line 784
#18 start_thread
at pthread_create.c line 335
#19 ??
at ../sysdeps/unix/sysv/linux/arm/clone.S line 86

Thread 1 (LWP 1371)

#0 __libc_do_syscall
at ../sysdeps/unix/sysv/linux/arm/libc-do-syscall.S line 46
#1 __lll_lock_wait
at lowlevellock.c line 46
#2 __GI___pthread_mutex_lock
at pthread_mutex_lock.c line 115
#3 post_activate
at ../../clone/gst/gstpad.c line 1015
#4 activate_mode_internal
at ../../clone/gst/gstpad.c line 1186
#5 gst_pad_activate_mode
at ../../clone/gst/gstpad.c line 1273
#6 gst_ghost_pad_activate_push_default
at ../../clone/gst/gstghostpad.c line 383
#7 gst_ghost_pad_activate_mode_default
at ../../clone/gst/gstghostpad.c line 449
#8 activate_mode_internal
at ../../clone/gst/gstpad.c line 1179
#9 gst_pad_set_active
at ../../clone/gst/gstpad.c line 1080
#10 activate_pads
at ../../clone/gst/gstbin.c line 2630
#11 gst_iterator_fold
at ../../clone/gst/gstiterator.c line 617
#12 iterator_activate_fold_with_resync
at ../../clone/gst/gstbin.c line 2653
#13 gst_bin_src_pads_activate
at ../../clone/gst/gstbin.c line 2687
#14 gst_bin_change_state_func
at ../../clone/gst/gstbin.c line 2881
#15 gst_element_change_state
at ../../clone/gst/gstelement.c line 2743
#16 gst_element_set_state_func
at ../../clone/gst/gstelement.c line 2697
#17 gst_adaptive_demux_stream_free
at ../../../../clone/gst-libs/gst/adaptivedemux/gstadaptivedemux.c line 1428
#18 g_list_foreach
at ../../glib-2.50.2/glib/glist.c line 1005
#19 g_list_free_full
at ../../glib-2.50.2/glib/glist.c line 220
#20 gst_adaptive_demux_reset
at ../../../../clone/gst-libs/gst/adaptivedemux/gstadaptivedemux.c line 812
#21 gst_adaptive_demux_change_state
at ../../../../clone/gst-libs/gst/adaptivedemux/gstadaptivedemux.c line 581
#22 gst_hls_demux_change_state
at ../../../clone/ext/hls/gsthlsdemux.c line 212
#23 gst_element_change_state
at ../../clone/gst/gstelement.c line 2743
#24 gst_element_set_state_func
at ../../clone/gst/gstelement.c line 2697
#25 gst_bin_element_set_state
at ../../clone/gst/gstbin.c line 2589
#26 gst_bin_change_state_func
at ../../clone/gst/gstbin.c line 2931
#27 gst_decode_bin_change_state
at ../../../gst-plugins-base-1.12.2/gst/playback/gstdecodebin2.c line 5342
#28 gst_element_change_state
at ../../clone/gst/gstelement.c line 2743
#29 gst_element_set_state_func
at ../../clone/gst/gstelement.c line 2697
#30 gst_bin_element_set_state
at ../../clone/gst/gstbin.c line 2589
#31 gst_bin_change_state_func
at ../../clone/gst/gstbin.c line 2931
#32 gst_uri_decode_bin_change_state
at ../../../gst-plugins-base-1.12.2/gst/playback/gsturidecodebin.c line 2737
#33 gst_element_change_state
at ../../clone/gst/gstelement.c line 2743
#34 gst_element_set_state_func
at ../../clone/gst/gstelement.c line 2697
#35 gst_bin_element_set_state
at ../../clone/gst/gstbin.c line 2589
#36 gst_bin_change_state_func
at ../../clone/gst/gstbin.c line 2931
#37 gst_element_change_state
at ../../clone/gst/gstelement.c line 2743
#38 gst_element_set_state_func
at ../../clone/gst/gstelement.c line 2697

Comment 1 Sebastian Dröge (slomo) 2017-08-08 09:13:57 UTC

Review of attachment 357171 [details] [review]:

Thanks for the patch. Can you provide it in "git format-patch" format and also explain in the commit message what the problem is and how this fixes it? Basically that you do things for not only the current streams but also for the newly prepared ones.

Comment 2 davecraig 2017-08-08 09:20:22 UTC

Created attachment 357176 [details] [review]
Patch in git format

Comment 3 davecraig 2017-08-08 09:24:48 UTC

Created attachment 357177 [details] [review]
Proposed patch

Comment 4 Sebastian Dröge (slomo) 2017-08-08 09:26:05 UTC

Thanks, merged! (The first one, but it seems to be equivalent)

Comment 5 davecraig 2017-08-08 09:27:21 UTC

(In reply to Sebastian Dröge (slomo) from comment #4)
> Thanks, merged! (The first one, but it seems to be equivalent)
They are - just trying to give you the format you asked for...! Thanks.

Comment 6 Sebastian Dröge (slomo) 2017-08-08 09:32:10 UTC

The last patch was in the correct format, for the previous one I fixed it up locally :) Thanks again!