[gstreamer-bugs] [Bug 635785] basesrc: fix deadlock

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=635785
  GStreamer | gstreamer (core) | unspecified

--- Comment #1 from Håvard Graff (hgr) <havard.graff at tandberg.com> 2010-11-25 16:25:32 UTC ---
More details:

<hgr> baseaudiosrc... 
<hgr> setting it between PLAYING and PAUSED really quickly, lots of times... 
<hgr> you get the following chain of events: 
<hgr> 1. PAUSED -> gst_base_src_wait_playing() is waiting in GST_LIVE_WAIT 
<hgr> 2. PLAYING -> gst_base_src_set_playing(live_play=TRUE) and the LIVE_LOCK
is taken 
<hgr> 3. the GST_LIVE_SIGNAL is signaled also inside gst_base_src_set_playing 
<hgr> 4. GST_LIVE_UNLOCK 
<hgr> Now the normal 5. would be that gst_base_src_wait_playing would
continue...after set_playing had let go of the lock... 
<hgr> however, very race, now 5. is PAUSED->
gst_base_src_set_playing(live_play=FALSE) and the LIVE_LOCK is taken *before*
the g_cond can take it again... 
<hgr> so then we get 6. GST_LIVE_UNLOCK inside gst_base_src_set_playing 
<hgr> and then 
<hgr> 7. gst_base_src_wait_playing continues 
<hgr> however, this ends up inside a 8. gst_ring_buffer_read () 
<wtay> oh 
<wtay> missing while around g_cond_wait() ? 
<hgr> and unless there is any buffer inside this read, it will block on
GST_RING_BUFFER_WAIT() 
<hgr> and now the state-change goes to PLAYING again... 
<hgr> and then noone will signal GST_RINGBUFFER_SIGNAL (that only happends when
going to PAUSED) 
<hgr> so you get a deadlock, where the state-changeing thread blocks on
gst_base_src_set_playing(live_play=TRUE) on the LIVE_LOCK, that is held by the
basesrc-thread, waiting for GST_RINGBUFFER_SIGNAL 
<hgr> the good news is that this only happens about every 1,5 million
state-changes, the bad news is that it *can* happen, and hence should be
fixed... :) 
<wtay> gst_base_src_wait_playing() is inside a loop that can only exit when
live_running = TRUE 
<wtay> or maybe audiosrc does something special... 
<hgr> however, it would probably not fix this issue, because
gst_base_src_set_playing needs to know that wait_playing has stopped waiting
before it is safe to return 
<wtay> indeed 
<wtay> the audio base class doesn't have a while loop 
<wtay> bad 
<hgr> ah, yeah, I know what while-loop you meant now... the one inside
gst_base_src... ! I am talking about audiobasesrc 
<wtay> it needs a while loop. either move it around in basesrc or add to
audiosrc 
<wtay> just move the while loop in wait-playing 
<wtay> it actually makes the method do what its name says :) 
<hgr> I guess... so you would keep waiting if it is not yet running? 
<wtay> yes 
<wtay> like in the get_range function 
<hgr> exactly 
<hgr> I'll try it :) 
<wtay> great 
<hgr> it looks perfect! I have never been able to run for this long before...
(*10) 
<wtay> yay 
<wtay> it makes sense, though 
<hgr> yup 
<hgr> I'll submit the patch asap 
<wtay> after a gcond, you always have to recheck the conditions that made you
go into the gcond 
<wtay> ..always.. 
<hgr> I now have *100 better than my best run 
<hgr> I say no more deadlock 
<wtay> easy fix

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.