From 47e303269d5bc9f1d00fe096da7af4c450ba854a Mon Sep 17 00:00:00 2001 From: Garg Date: Fri, 5 Sep 2014 09:54:10 -0700 Subject: [PATCH] audiobasesink: Fix deadlock caused by holding object lock while calling clock functions Issue: During a PAUSED->PLAYING transition when we are rendering an audio buffer in AudioBaseSink we make adjustments to the sink's provided clock i.e. fix clock calibration using the external pipeline clock, within "gst_audio_base_sink_sync_latency function inside gstaudiobasesink.c". For the calibration adjustment we need to get the sink clock time using "gst_audio_clock_get_time". But before calling "gst_audio_clock_get_time" we acquire the Object Lock on the Sink. If sink is a pulsesink, "gst_audio_clock_get_time" internally calls "gst_pulsesink_get_time" which needs to acquire Pulse Audio Main Loop Lock before querying Pulse Audio for its stream time using "pa_stream_get_time". Please see "gst_pulsesink_get_time in pulsesink.c". So the situation here is we have acquired the Object lock on Sink and need PA Main Loop Lock. Now Pulse Audio Main Thread itself might be in the process of posting a stream status message after Paused to Playing transition which in turn acquires the PA Main loop lock and needs the Object Lock on Pulse Sink. This causes a deadlock with the earlier render thread. Fix: Do not acquire the object Lock on Sink before querying the time on PulseSink clock. This is similar to the way we have used get_time at other places in the code. Acquire it after the get_time call. This way PA Main loop will be able to post its stream status message by acquiring the Sink Object lock and will eventually release its Main Loop lock needed for gst_pulsesink_get_time to continue. https://bugzilla.gnome.org/show_bug.cgi?id=736071 --- gst-libs/gst/audio/gstaudiobasesink.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/gst-libs/gst/audio/gstaudiobasesink.c b/gst-libs/gst/audio/gstaudiobasesink.c index 26f71347ea..4be9988153 100644 --- a/gst-libs/gst/audio/gstaudiobasesink.c +++ b/gst-libs/gst/audio/gstaudiobasesink.c @@ -1464,15 +1464,21 @@ gst_audio_base_sink_sync_latency (GstBaseSink * bsink, GstMiniObject * obj) * yet. if some other error occures, we continue. */ } while (status == GST_CLOCK_UNSCHEDULED); - GST_OBJECT_LOCK (sink); GST_DEBUG_OBJECT (sink, "latency synced"); + /* Do not acquire the sink object lock before trying to get time on the Sink. + * The get_time call may need to acquire the pulse audio mainloop lock. This can + * cause a deadlock with the Pulse Audio main loop thread which in turn has + * acquired its mainloop lock and now needs to acquire Object lock on the sink. */ + itime = gst_audio_clock_get_time (sink->provided_clock); + itime = gst_audio_clock_adjust (sink->provided_clock, itime); + + GST_OBJECT_LOCK (sink); + /* when we prerolled in time, we can accurately set the calibration, * our internal clock should exactly have been the latency (== the running * time of the external clock) */ etime = GST_ELEMENT_CAST (sink)->base_time + time; - itime = gst_audio_clock_get_time (sink->provided_clock); - itime = gst_audio_clock_adjust (sink->provided_clock, itime); if (status == GST_CLOCK_EARLY) { /* when we prerolled late, we have to take into account the lateness */