pulsesink: Make emitting stream status messages synchronous

The stream status messages are emitted in the PA mainloop thread, which
means the mainloop lock is taken, followed by the Gst object lock (by
gst_element_post_message()). In all other locations, the order of
locking is reversed (this is unavoidable in a bunch of cases where the
object lock is taken by GstBaseSink or GstAudioBaseSink, and then we get
control to take the mainloop lock).

The only way to guarantee that the defer callback for stream status
messages doesn't deadlock is to either stop posting those messages, or
make sure that the message emission is completed before we proceed to
any point that might take the object lock before the mainloop lock
(which is what we do after this patch).

https://bugzilla.gnome.org/show_bug.cgi?id=736071

diff --git a/ext/pulse/pulsesink.c b/ext/pulse/pulsesink.c
index e580afc..e77ce8d 100644 (file)
--- a/ext/pulse/pulsesink.c
+++ b/ext/pulse/pulsesink.c
@@ -1209,6 +1209,14 @@ gst_pulseringbuffer_start (GstAudioRingBuffer * buf)
       g_atomic_int_get (&GST_AUDIO_BASE_SINK (psink)->eos_rendering))
     gst_pulsering_set_corked (pbuf, FALSE, FALSE);
 
+  /* Wait for the stream status message to be posted. This needs to be done
+   * synchronously because the callback will take the mainloop lock
+   * (implicitly) and then take the GST_OBJECT_LOCK. Everywhere else, we take
+   * the locks in the reverse order, so not doing this synchronously could
+   * cause a deadlock. */
+  GST_DEBUG_OBJECT (psink, "waiting for stream status (ENTER) to be posted");
+  pa_threaded_mainloop_wait (mainloop);
+
   pa_threaded_mainloop_unlock (mainloop);
 
   return TRUE;
@@ -1315,6 +1323,14 @@ cleanup:
   pa_mainloop_api_once (pa_threaded_mainloop_get_api (mainloop),
       mainloop_leave_defer_cb, psink);
 
+  /* Wait for the stream status message to be posted. This needs to be done
+   * synchronously because the callback will take the mainloop lock
+   * (implicitly) and then take the GST_OBJECT_LOCK. Everywhere else, we take
+   * the locks in the reverse order, so not doing this synchronously could
+   * cause a deadlock. */
+  GST_DEBUG_OBJECT (psink, "waiting for stream status (LEAVE) to be posted");
+  pa_threaded_mainloop_wait (mainloop);
+
   pa_threaded_mainloop_unlock (mainloop);
 
   return res;