GNOME Bugzilla – Bug 684312
rtspsrc: mutex blocks going to NULL state
Last modified: 2012-12-10 14:49:43 UTC
Created attachment 224654 [details] gstrtspsrc stream mutex lock We have ran into a mutex lock when rtspsrc element tries to go to NULL state. We have been able to reproduce this multiple times, but unfortunately I do not have a simple test to attach here. However, I attach the backtrace where we can see the rtspsrc element blocked in the stream mutex when going to NULL state (in gst_rtspsrc_stop).
Created attachment 224655 [details] [review] flush connection before locking mutex At gst_rtspsrc_stop we flush the connection before GST_RTSP_STREAM_LOCK (src). This will really make sure we don't get blocked, as the loop (if running) will exit and the lock will be released.
By the way, after the patch, we seem not to be able to reproduce the blocking.
Created attachment 224786 [details] [review] do not run commands if changing to NULL state We have been able to reproduce this again, this time more easily. I finally understood what's going on. We are setting the pipeline to NULL from our application thread. The problem is that gst_rtspsrc_thread might also wanted to try changing the state (e.g. gst_rtspsrc_play). However, as we are going to NULL and gst_rtspsrc_stop is called inside a state change, everything blocks. From the commit log: we avoid running any command if we are in the middle of a NULL state change. NULL state change might happen in a different thread, so we don't want to change the state from gst_rtspsrc_thread (e.g. in gst_rtspsrc_play).
So, last patch didn't work. This one is being hard and I don't see a way to fix that. Basically, the problem is that the element can change to NULL state while the gstrtspsrc thread is trying to set it to PLAYING. Both threads just block. The NULL thread is waiting for the task to complete and the PLAYING thread is waiting for the NULL state change to complete.
Do you have a test program to reproduce it by any chance?
(In reply to comment #5) > Do you have a test program to reproduce it by any chance? Unfortunately no, but I'll see if I can come up with one.
Created attachment 224885 [details] simple deadlock test I attach an example that causes this deadlock problem. There's a little trick though, as I have added a sleep in gstrtspsrc.c and in the test to force this situation: case CMD_PLAY: g_usleep (5 * G_USEC_PER_SEC); ret = gst_rtspsrc_play (src, &src->segment, TRUE); So, I am not sure if you will consider this as a valid test. The other point is that may be we should not change to NULL state just after going to PLAYING... But I guess, it's legal, right? ========== Run test-launch example in gst-rtsp-server with: GST_DEBUG=3 ./test-launch "( videotestsrc ! x264enc ! rtph264pay name=pay0 pt=96 )" Then, for the client: $ make $ gdb --args ./test-bug-684312 rtsp://localhost:8554/test
(In reply to comment #7) > Created an attachment (id=224885) [details] > simple deadlock test > > I attach an example that causes this deadlock problem. There's a little trick > though, as I have added a sleep in gstrtspsrc.c and in the test to force this > situation: > > case CMD_PLAY: > g_usleep (5 * G_USEC_PER_SEC); > ret = gst_rtspsrc_play (src, &src->segment, TRUE); > Didn't mention, but you have to add it too.
Created attachment 225282 [details] [review] do not change state to PLAYING if in the middle of another state change I didn't see another way to do this it solves the deadlock when going to NULL state. Not sure if this will cause any side effects. So far, it's working fine with a stress test of opening/closing rtspsrc continuously. From the commit log: * gst/rtsp/gstrtspsrc.c (gst_rtspsrc_play): state change might be happening in the application thread, so we don't change the state to PLAYING in the gstrtspsrc thread unless it is safe. A specific case is when chaning the state to NULL from the application thread. This will synchronously try to stop the task (with the element state lock acquired), but we will try a gst_element_set_state from gstrtspsrc thread which will block on the element state lock causing a deadlock.
Any comments on this patch? It has been working successfully for a couple of months now.
commit 3503aef946649da39e9b423adebe173f7a692b89 Author: Aleix Conchillo Flaque <aleix@oblong.com> Date: Thu Sep 27 12:17:58 2012 -0700 rtspsrc: do not change state to PLAYING if currently chaning state * gst/rtsp/gstrtspsrc.c (gst_rtspsrc_play): state change might be happening in the application thread, so we don't change the state to PLAYING in the gstrtspsrc thread unless it is safe. A specific case is when chaning the state to NULL from the application thread. This will synchronously try to stop the task (with the element state lock acquired), but we will try a gst_element_set_state from gstrtspsrc thread which will block on the element state lock causing a deadlock. https://bugzilla.gnome.org/show_bug.cgi?id=684312
Great, thanks! Can this be also applied to 0.10 branch?
(In reply to comment #12) > Great, thanks! Can this be also applied to 0.10 branch? Done! but only because it cleanup cherry picked..