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 776039 - queue: deadlock between sink query and state change
queue: deadlock between sink query and state change
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal blocker
: 1.10.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 774854 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2016-12-13 10:30 UTC by Jesper Larsen
Modified: 2016-12-22 12:48 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Test case (1.07 KB, text/x-csrc)
2016-12-13 10:30 UTC, Jesper Larsen 		  Details
queue/queue2: Ensure that the streaming thread is unlocked after deactivating the srcpad (1.71 KB, patch)
2016-12-13 18:02 UTC, Sebastian Dröge (slomo) 		committed Details | Review

Description Jesper Larsen 2016-12-13 10:30:55 UTC 
Created attachment 341864 [details]
Test case

A queue can deadlock, if it is handling a sink query and a state change at the same time.

This seems to be a regression introduced by 722ad087338520047241a319a506e464017bf0da discussed in https://bugzilla.gnome.org/show_bug.cgi?id=763496

I have attached a test application. With the current master (49f1f35), I hit a repeatable deadlock within seconds. If I revert 722ad087338520047241a319a506e464017bf0da the deadlock does not happen.

A trace of the deadlock with gstreamer sources on 49f1f35:

+ Trace 236975

Thread 169 (Thread 0x7fffeffff700 (LWP 26643))

  • #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 115
  • #2 gst_queue_sink_activate_mode
    at gstqueue.c line 1687
  • #3 activate_mode_internal
    at gstpad.c line 1178
  • #4 gst_pad_set_active
    at gstpad.c line 1079
  • #5 activate_pads
    at gstelement.c line 2828
  • #6 gst_iterator_fold
    at gstiterator.c line 616
  • #7 iterator_activate_fold_with_resync
    at gstelement.c line 2852
  • #8 gst_element_pads_activate
    at gstelement.c line 2896
  • #9 gst_element_change_state_func
    at gstelement.c line 2962
  • #10 gst_element_change_state
    at gstelement.c line 2740
  • #11 gst_element_set_state_func
    at gstelement.c line 2694
  • #12 gst_bin_element_set_state
    at gstbin.c line 2619
  • #13 gst_bin_change_state_func
    at gstbin.c line 2961
  • #14 gst_element_change_state
    at gstelement.c line 2740
  • #15 gst_element_set_state_func
    at gstelement.c line 2694
  • #16 close_func
  • #17 g_thread_proxy
    at gthread.c line 784
  • #18 start_thread
    at pthread_create.c line 333
  • #19 clone
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S line 105

Thread 6 (Thread 0x7fffef7fe700 (LWP 26480))

  • #0 syscall
    at ../sysdeps/unix/sysv/linux/x86_64/syscall.S line 38
  • #1 g_cond_wait_until
    at gthread-posix.c line 1442
  • #2 g_async_queue_pop_intern_unlocked
    at gasyncqueue.c line 422
  • #3 g_async_queue_timeout_pop
    at gasyncqueue.c line 543
  • #4 g_thread_pool_wait_for_new_pool
    at gthreadpool.c line 167
  • #5 g_thread_pool_thread_proxy
    at gthreadpool.c line 364
  • #6 g_thread_proxy
    at gthread.c line 784
  • #7 start_thread
    at pthread_create.c line 333
  • #8 clone
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S line 105

Thread 3 (Thread 0x7ffff49ad700 (LWP 26477))

  • #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 115
  • #2 gst_pad_send_event_unchecked
    at gstpad.c line 5570
  • #3 gst_pad_push_event_unchecked
    at gstpad.c line 5262
  • #4 push_sticky
    at gstpad.c line 3805
  • #5 events_foreach
    at gstpad.c line 603
  • #6 check_sticky
    at gstpad.c line 3862
  • #7 gst_pad_push_event
    at gstpad.c line 5393
  • #8 gst_base_src_send_stream_start
    at gstbasesrc.c line 886
  • #9 gst_base_src_send_stream_start
    at gstbasesrc.c line 2703
  • #10 gst_base_src_loop
    at gstbasesrc.c line 2693
  • #11 gst_task_func
    at gsttask.c line 334
  • #12 g_thread_pool_thread_proxy
    at gthreadpool.c line 307
  • #13 g_thread_proxy
    at gthread.c line 784
  • #14 start_thread
    at pthread_create.c line 333
  • #15 clone
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S line 105

Thread 1 (Thread 0x7ffff7fc6700 (LWP 26471))

  • #0 syscall
    at ../sysdeps/unix/sysv/linux/x86_64/syscall.S line 38
  • #1 g_cond_wait
    at gthread-posix.c line 1395
  • #2 gst_queue_handle_sink_query
    at gstqueue.c line 1048
  • #3 gst_pad_query
    at gstpad.c line 3948
  • #4 main 






Comment 1


Sebastian Dröge (slomo)





          2016-12-13 11:21:24 UTC
        



I can confirm, but not sure why.






Comment 2


Sebastian Dröge (slomo)





          2016-12-13 17:19:31 UTC
        



Reason is that stopping a task fast enough after starting it, will cause the loop function to be never called. And we do shutdown from src to sink, that is we require the loop function to unlock all things (otherwise we end up with the above mentioned bug again, and unref a query that downstream is still using).






Comment 3


Thibault Saunier





          2016-12-13 17:25:50 UTC
        



This is a duplicate of https://bugzilla.gnome.org/show_bug.cgi?id=774854?






Comment 4


Sebastian Dröge (slomo)





          2016-12-13 17:38:37 UTC
        



Possible. I have a fix, will put here after some more testing and then you can tell me ;)






Comment 5


Sebastian Dröge (slomo)





          2016-12-13 18:02:44 UTC
        



Created attachment 341903 [details] [review]
queue/queue2: Ensure that the streaming thread is unlocked after deactivating the srcpad

It might happen that the srcpad task function is never called at all, in
which case unlocking everything from there will never happen.

Make sure to unlock everything another time after the task function is
definitely stopped.






Comment 6


Jesper Larsen





          2016-12-13 18:34:47 UTC
        



The proposed patch seems to work on the test case, as well as the application where I originally saw the issue.






Comment 7


Sebastian Dröge (slomo)





          2016-12-13 19:05:14 UTC
        



Multiqueue needs some unrelated fixes still

commit 3fab00b46aad44e0375988fd61e13e900fa114a4
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Tue Dec 13 20:51:17 2016 +0200

    queue/queue2: Protect against spurious condition variable wakeups
    
    Make sure that we only wake up when we have to flush, or when this
    specific query was handled.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=776039

commit 33c239828bd0563114fddb4fece75f95a986d41e
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Tue Dec 13 20:00:55 2016 +0200

    queue/queue2: Ensure that the streaming thread is unlocked after deactivating the srcpad
    
    It might happen that the srcpad task function is never called at all, in
    which case unlocking everything from there will never happen.
    
    Make sure to unlock everything another time after the task function is
    definitely stopped.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=776039






Comment 8


Thibault Saunier





          2016-12-13 19:42:44 UTC
        



*** Bug 774854 has been marked as a duplicate of this bug. ***