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 721253 - multiqueue: May cause hanging if shut down while handling a serialized query
multiqueue: May cause hanging if shut down while handling a serialized query
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
1.0.5
Other Linux
: Normal critical
: 1.2.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-12-31 01:59 UTC by zhangyanping
Modified: 2014-01-06 10:01 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
diff (450 bytes, patch)
2014-01-02 02:53 UTC, zhangyanping
needs-work Details | Review
fix multiqueue hanging when stopping (824 bytes, patch)
2014-01-03 06:04 UTC, zhangyanping
committed Details | Review

Description zhangyanping 2013-12-31 01:59:37 UTC
I use playbin to playing a http stream.
When the pipeline did not construct finished, I called  gst_element_set_state(playbin, GST_STATE_NULL);
At this time, application hang.
This is call stack of threads that cause haning.

======================

Thread 5 (Thread 0x2b98a480 (LWP 22769))

  • #0 __lll_lock_wait
    from /lib/libpthread.so.0
  • #1 pthread_mutex_lock
    from /lib/libpthread.so.0
  • #2 g_rec_mutex_lock
    at gthread-posix.c line 377
  • #3 post_activate
    at gstpad.c line 917
  • #4 gst_pad_activate_mode
    at gstpad.c line 1121
  • #5 gst_pad_set_active
    at gstpad.c line 989
  • #6 activate_pads
    at gstelement.c line 2679
  • #7 gst_iterator_fold
    at gstiterator.c line 614
  • #8 iterator_activate_fold_with_resync
    at gstelement.c line 2699
  • #9 gst_element_pads_activate
    at gstelement.c line 2744
  • #10 gst_element_change_state_func
    at gstelement.c line 2807
  • #11 gst_multi_queue_change_state
    at gstmultiqueue.c line 723
  • #12 gst_element_change_state
    at gstelement.c line 2594
  • #13 gst_element_set_state_func
    at gstelement.c line 2550
  • #14 gst_element_set_state
    at gstelement.c line 2451
  • #15 gst_bin_element_set_state
    at gstbin.c line 2308
  • #16 gst_bin_change_state_func
    at gstbin.c line 2612
  • #17 gst_pipeline_change_state
    at gstpipeline.c line 468
  • #18 gst_play_bin_change_state
    at gstplaybin3.c line 1913
  • #19 gst_element_change_state
    at gstelement.c line 2594
  • #20 gst_element_continue_state
    at gstelement.c line 2305
  • #21 gst_element_change_state
    at gstelement.c line 2631
  • #22 gst_element_set_state_func
    at gstelement.c line 2550
  • #23 gst_element_set_state
    at gstelement.c line 2451

===========================

I found the root cause is at function gst_multi_queue_sink_query,

g_cond_wait (&sq->query_handled, &mq->qlock);

It will wait the query condition to be signaled. But because the whole pileline did not construct finished and want to stop immediately, the sq->query_handled have no chance to be signaled.

So at function gst_multi_queue_change_state


    case GST_STATE_CHANGE_PAUSED_TO_READY:{
      GList *tmp;

      /* Un-wait all waiting pads */
      GST_MULTI_QUEUE_MUTEX_LOCK (mqueue);
      for (tmp = mqueue->queues; tmp; tmp = g_list_next (tmp)) {
        sq = (GstSingleQueue *) tmp->data;
        sq->flushing = TRUE;
        g_cond_signal (&sq->turn);

       g_cond_signal (&sq->query_handled);    //I add this line ===========
      }
      GST_MULTI_QUEUE_MUTEX_UNLOCK (mqueue);
      break;
    }

And I test serveral hours to do quick switch:
1. create playin
2. set state playing
3. sleep 100 ms
4. set state null

This will be OK.

//==========================
And I check the codes of 1.2.1, there are some other lines will call g_cond_signal (&sq->query_handled).

But I think add it to gst_multi_queue_change_state is better and safer.
Comment 1 zhangyanping 2013-12-31 02:04:10 UTC
Anther thread callstack:

Thread 3 (Thread 0x2d6fe480 (LWP 22776))

  • #0 pthread_cond_wait
    from /lib/libpthread.so.0
  • #1 g_cond_wait
    at gthread-posix.c line 746
  • #2 gst_multi_queue_sink_query
    at gstmultiqueue.c line 1614
  • #3 gst_pad_query
    at gstpad.c line 3548
  • #4 gst_pad_peer_query
    at gstpad.c line 3684
  • #5 qtdemux_do_allocation
    at qtdemux.c line 5130
  • #6 gst_qtdemux_add_stream
    at qtdemux.c line 5343
  • #7 qtdemux_expose_streams
    at qtdemux.c line 7925
  • #8 gst_qtdemux_chain
    at qtdemux.c line 4310
  • #9 gst_pad_chain_data_unchecked
    at gstpad.c line 3790
  • #10 gst_pad_push_data
    at gstpad.c line 4011
  • #11 gst_pad_push
    at gstpad.c line 4116
  • #12 gst_type_find_element_chain
    at gsttypefindelement.c line 828
  • #13 gst_pad_chain_data_unchecked
    at gstpad.c line 3790
  • #14 gst_pad_push_data
    at gstpad.c line 4011
  • #15 gst_pad_push
    at gstpad.c line 4116
  • #16 gst_queue_push_one
    at gstqueue.c line 1057
  • #17 gst_queue_loop
    at gstqueue.c line 1175
  • #18 gst_task_func
    at gsttask.c line 316

Comment 2 Sebastian Dröge (slomo) 2013-12-31 09:47:18 UTC
You also need to set sq->last_query=FALSE here. Can you provide this patch in "git format-patch" format? For this commit the change locally (make sure to set up your real name and mail address in git) and then do "git format-patch -1".
Comment 3 zhangyanping 2014-01-02 02:53:41 UTC
Created attachment 265118 [details] [review]
diff
Comment 4 zhangyanping 2014-01-02 02:54:45 UTC
(In reply to comment #3)
> Created an attachment (id=265118) [details] [review]
> diff

Index: gstmultiqueue.c
===================================================================
--- gstmultiqueue.c	(revision 41444)
+++ gstmultiqueue.c	(working copy)
@@ -712,6 +712,9 @@
         sq = (GstSingleQueue *) tmp->data;
         sq->flushing = TRUE;
         g_cond_signal (&sq->turn);
+        
+        sq->last_query = FALSE;
+        g_cond_signal (&sq->query_handled);
       }
       GST_MULTI_QUEUE_MUTEX_UNLOCK (mqueue);
       break;
Comment 5 Tim-Philipp Müller 2014-01-02 08:02:43 UTC
Could you please attach the patch in git format-patch format?
Comment 6 Sebastian Dröge (slomo) 2014-01-02 09:19:18 UTC
Similar change might be necessary in the other queues too. Should be checked before pushing this.
Comment 7 zhangyanping 2014-01-03 06:04:51 UTC
Created attachment 265201 [details] [review]
fix multiqueue hanging when stopping

signal query_handled when stopping multiqueue
Comment 8 zhangyanping 2014-01-03 06:07:41 UTC
(In reply to comment #7)
> Created an attachment (id=265201) [details] [review]
> fix multiqueue hanging when stopping
> 
> signal query_handled when stopping multiqueue

I checked gstqueue and gstqueue2. And I think queue and queue2 don't have this problem.
Comment 9 Sebastian Dröge (slomo) 2014-01-03 10:16:04 UTC
Please also provide a proper commit message like the following in the future :)

commit 1de533735b93ec8cbad346267f99e6519d0e3cc8
Author: YanpingZhang <zhangyanping210@163.com>
Date:   Fri Jan 3 11:47:23 2014 +0800

    multiqueue: Fix hanging if shut down while handling a serialized query
    
    https://bugzilla.gnome.org/show_bug.cgi?id=721253