wpe: Fix race condition on teardown
authorThibault Saunier <tsaunier@igalia.com>
Wed, 1 Sep 2021 21:35:45 +0000 (17:35 -0400)
committerGStreamer Marge Bot <gitlab-merge-bot@gstreamer-foundation.org>
Fri, 3 Sep 2021 15:56:31 +0000 (15:56 +0000)
There was a race when going to PAUSED while pushing a buffer to the
pipeline process (where we weren't even cancelling anything).

This rework base all the cancellation around the GCancellable
"cancelled" signal trying to ensure that the streaming thread will not
block once a cancel operation happens.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/-/merge_requests/2504>

ext/wpe/wpe-extension/gstwpeaudiosink.c

index d5d8a94..66a6c4d 100644 (file)
@@ -186,11 +186,28 @@ unlock (GstBaseSink * sink)
   GstWpeAudioSink *self = GST_WPE_AUDIO_SINK (sink);
 
   g_cancellable_cancel (self->cancellable);
+
+  return TRUE;
+}
+
+static gboolean
+unlock_stop (GstBaseSink * sink)
+{
+  GstWpeAudioSink *self = GST_WPE_AUDIO_SINK (sink);
+  GCancellable *cancellable = self->cancellable;
+
+  self->cancellable = g_cancellable_new ();
+  g_object_unref (cancellable);
+
+  return TRUE;
+}
+
+static void
+_cancelled_cb (GCancellable * _cancellable, GstWpeAudioSink * self)
+{
   g_mutex_lock (&self->buf_lock);
   g_cond_broadcast (&self->buf_cond);
   g_mutex_unlock (&self->buf_lock);
-
-  return TRUE;
 }
 
 static gboolean
@@ -205,7 +222,7 @@ stop (GstBaseSink * sink)
   }
 
   /* Stop processing and claim buffers back */
-  unlock (sink);
+  g_cancellable_cancel (self->cancellable);
 
   GST_DEBUG_OBJECT (sink, "Stopping %d", self->id);
   gst_wpe_extension_send_message (webkit_user_message_new ("gstwpe.stop",
@@ -220,9 +237,22 @@ change_state (GstElement * element, GstStateChange transition)
   GstWpeAudioSink *self = GST_WPE_AUDIO_SINK (element);
 
   switch (transition) {
+    case GST_STATE_CHANGE_PAUSED_TO_PLAYING:
+    {
+      if (g_cancellable_is_cancelled (self->cancellable)) {
+        GCancellable *cancellable = self->cancellable;
+        self->cancellable = g_cancellable_new ();
+
+        g_object_unref (cancellable);
+      }
+      break;
+    }
     case GST_STATE_CHANGE_PLAYING_TO_PAUSED:
+      g_cancellable_cancel (self->cancellable);
+
       gst_wpe_extension_send_message (webkit_user_message_new ("gstwpe.pause",
-              g_variant_new_uint32 (self->id)), self->cancellable, NULL, NULL);
+              g_variant_new_uint32 (self->id)), NULL, NULL, NULL);
+
       break;
     default:
       break;
@@ -250,6 +280,8 @@ gst_wpe_audio_sink_init (GstWpeAudioSink * self)
   g_return_if_fail (pad_template != NULL);
 
   self->cancellable = g_cancellable_new ();
+  g_cancellable_connect (self->cancellable,
+      G_CALLBACK (_cancelled_cb), self, NULL);
 }
 
 static void
@@ -273,6 +305,7 @@ gst_wpe_audio_sink_class_init (GstWpeAudioSinkClass * klass)
   gstelement_class->change_state = GST_DEBUG_FUNCPTR (change_state);
   gstbasesink_class->stop = GST_DEBUG_FUNCPTR (stop);
   gstbasesink_class->unlock = GST_DEBUG_FUNCPTR (unlock);
+  gstbasesink_class->unlock_stop = GST_DEBUG_FUNCPTR (unlock_stop);
   gstbasesink_class->render = GST_DEBUG_FUNCPTR (render);
   gstbasesink_class->set_caps = GST_DEBUG_FUNCPTR (set_caps);
 }