From 15caeb4ac91fd0352ab9efe4cee1e6e343763bd2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alicia=20Boya=20Garc=C3=ADa?= <aboya@igalia.com>
Date: Sat, 4 May 2019 03:54:44 +0200
Subject: [PATCH] gstelement: fix deadlock in gst_element_add_pad() when
 >=PAUSED

gst_element_add_pad() is supposed to activate the pad if the element
state is >= PAUSED and the pad is not already active.

Unfortunately, before this patch, the activation was performed while the
element lock was still taken, which ended causing a deadlock in
gst_pad_start_task() as it attempted to post `stream-status` message in
the element, which also requires the element lock.

Elements could work around this bug by activating the pad manually
before adding it to the element.

This patch fixes the problem by performing pad activation only after the
element lock has been released.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/3635>
---
 subprojects/gstreamer/gst/gstelement.c        | 10 ++--
 .../gstreamer/tests/check/gst/gstelement.c    | 50 +++++++++++++++++++
 2 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/subprojects/gstreamer/gst/gstelement.c b/subprojects/gstreamer/gst/gstelement.c
index 637924c2dc..154b612fba 100644
--- a/subprojects/gstreamer/gst/gstelement.c
+++ b/subprojects/gstreamer/gst/gstelement.c
@@ -747,6 +747,7 @@ gst_element_add_pad (GstElement * element, GstPad * pad)
 {
   gchar *pad_name;
   gboolean active;
+  gboolean should_activate;
 
   g_return_val_if_fail (GST_IS_ELEMENT (element), FALSE);
   g_return_val_if_fail (GST_IS_PAD (pad), FALSE);
@@ -771,10 +772,8 @@ gst_element_add_pad (GstElement * element, GstPad * pad)
     goto had_parent;
 
   /* check for active pads */
-  if (!active && (GST_STATE (element) > GST_STATE_READY ||
-          GST_STATE_NEXT (element) == GST_STATE_PAUSED)) {
-    gst_pad_set_active (pad, TRUE);
-  }
+  should_activate = !active && (GST_STATE (element) > GST_STATE_READY ||
+      GST_STATE_NEXT (element) == GST_STATE_PAUSED);
 
   g_free (pad_name);
 
@@ -796,6 +795,9 @@ gst_element_add_pad (GstElement * element, GstPad * pad)
   element->pads_cookie++;
   GST_OBJECT_UNLOCK (element);
 
+  if (should_activate)
+    gst_pad_set_active (pad, TRUE);
+
   /* emit the PAD_ADDED signal */
   g_signal_emit (element, gst_element_signals[PAD_ADDED], 0, pad);
   GST_TRACER_ELEMENT_ADD_PAD (element, pad);
diff --git a/subprojects/gstreamer/tests/check/gst/gstelement.c b/subprojects/gstreamer/tests/check/gst/gstelement.c
index 0131a3740e..a1bf2aacdc 100644
--- a/subprojects/gstreamer/tests/check/gst/gstelement.c
+++ b/subprojects/gstreamer/tests/check/gst/gstelement.c
@@ -83,6 +83,55 @@ GST_START_TEST (test_add_pad_unref_element)
 
 GST_END_TEST;
 
+static void
+test_add_pad_while_paused_dummy_task (void *user_data)
+{
+  GstPad *pad = (GstPad *) user_data;
+  gst_pad_pause_task (pad);
+}
+
+static gboolean
+test_add_pad_while_paused_pad_activatemode (GstPad * pad, GstObject * parent,
+    GstPadMode mode, gboolean active)
+{
+  *(gboolean *) pad->activatemodedata = active;
+  fail_unless (mode == GST_PAD_MODE_PUSH);
+  if (active)
+    gst_pad_start_task (pad, test_add_pad_while_paused_dummy_task, pad, NULL);
+  else
+    gst_pad_stop_task (pad);
+  return TRUE;
+}
+
+GST_START_TEST (test_add_pad_while_paused)
+{
+  GstElement *e;
+  GstPad *p;
+  gboolean active = FALSE;
+
+  e = gst_element_factory_make ("fakesrc", "source");
+  gst_element_set_state (e, GST_STATE_PAUSED);
+  {
+    GstPad *old_pad = gst_element_get_static_pad (e, "src");
+    gst_element_remove_pad (e, old_pad);
+    gst_object_unref (old_pad);
+  }
+
+  p = gst_pad_new ("dynamic", GST_PAD_SRC);
+  gst_pad_set_activatemode_function_full (p,
+      test_add_pad_while_paused_pad_activatemode, (void *) &active, NULL);
+
+  fail_if (active);
+  gst_element_add_pad (e, p);
+  fail_if (!active);
+  gst_element_set_state (e, GST_STATE_NULL);
+  fail_if (active);
+
+  gst_object_unref (e);
+}
+
+GST_END_TEST;
+
 GST_START_TEST (test_error_no_bus)
 {
   GstElement *e;
@@ -921,6 +970,7 @@ gst_element_suite (void)
   suite_add_tcase (s, tc_chain);
   tcase_add_test (tc_chain, test_add_remove_pad);
   tcase_add_test (tc_chain, test_add_pad_unref_element);
+  tcase_add_test (tc_chain, test_add_pad_while_paused);
   tcase_add_test (tc_chain, test_error_no_bus);
   tcase_add_test (tc_chain, test_link);
   tcase_add_test (tc_chain, test_link_no_pads);