ghostpad: fix race-condition while tearing down

An upstream query will take a ref on the internal proxypad, and can
hence end up owning the last reference to that pad, causing a crash.

diff --git a/gst/gstghostpad.c b/gst/gstghostpad.c
index 478d274..60a265b 100644 (file)
--- a/gst/gstghostpad.c
+++ b/gst/gstghostpad.c
@@ -506,13 +506,16 @@ gst_ghost_pad_dispose (GObject * object)
 
   GST_OBJECT_LOCK (pad);
   internal = GST_PROXY_PAD_INTERNAL (pad);
+  if (internal) {
+    gst_pad_set_activatemode_function (internal, NULL);
 
-  gst_pad_set_activatemode_function (internal, NULL);
+    GST_PROXY_PAD_INTERNAL (pad) = NULL;
+    GST_PROXY_PAD_INTERNAL (internal) = NULL;
 
-  /* disposes of the internal pad, since the ghostpad is the only possible object
-   * that has a refcount on the internal pad. */
-  gst_object_unparent (GST_OBJECT_CAST (internal));
-  GST_PROXY_PAD_INTERNAL (pad) = NULL;
+    /* disposes of the internal pad, since the ghostpad is the only possible object
+     * that has a refcount on the internal pad. */
+    gst_object_unparent (GST_OBJECT_CAST (internal));
+  }
 
   GST_OBJECT_UNLOCK (pad);
 
@@ -831,7 +834,12 @@ gst_ghost_pad_set_target (GstGhostPad * gpad, GstPad * newtarget)
 
   g_return_val_if_fail (GST_IS_GHOST_PAD (gpad), FALSE);
   g_return_val_if_fail (GST_PAD_CAST (gpad) != newtarget, FALSE);
-  g_return_val_if_fail (newtarget != GST_PROXY_PAD_INTERNAL (gpad), FALSE);
+
+  if (newtarget == GST_PROXY_PAD_INTERNAL (gpad)) {
+    GST_WARNING_OBJECT (gpad, "Target has already been set to %s:%s",
+        GST_DEBUG_PAD_NAME (newtarget));
+    return FALSE;
+  }
 
   GST_OBJECT_LOCK (gpad);
   internal = GST_PROXY_PAD_INTERNAL (gpad);

diff --git a/tests/check/gst/gstghostpad.c b/tests/check/gst/gstghostpad.c
index 1a56ed3..94ec14f 100644 (file)
--- a/tests/check/gst/gstghostpad.c
+++ b/tests/check/gst/gstghostpad.c
@@ -1321,6 +1321,51 @@ GST_START_TEST (test_activate_sink_switch_mode)
 
 GST_END_TEST;
 
+static gboolean thread_running;
+static gpointer
+send_query_to_pad_func (GstPad * pad)
+{
+  GstQuery *query = gst_query_new_latency ();
+
+  while (thread_running) {
+    gst_pad_peer_query (pad, query);
+    g_thread_yield ();
+  }
+
+  gst_query_unref (query);
+  return NULL;
+}
+
+GST_START_TEST (test_stress_upstream_queries_while_tearing_down)
+{
+  GThread *query_thread;
+  gint i;
+  GstPad *pad = gst_pad_new ("sink", GST_PAD_SINK);
+  gst_pad_set_active (pad, TRUE);
+
+  thread_running = TRUE;
+  query_thread = g_thread_new ("queries",
+      (GThreadFunc) send_query_to_pad_func, pad);
+
+  for (i = 0; i < 1000; i++) {
+    GstPad *ghostpad = gst_ghost_pad_new ("ghost-sink", pad);
+    gst_pad_set_active (ghostpad, TRUE);
+
+    g_thread_yield ();
+
+    gst_ghost_pad_set_target (GST_GHOST_PAD_CAST (ghostpad), NULL);
+    gst_pad_set_active (pad, FALSE);
+    gst_object_unref (ghostpad);
+  }
+
+  thread_running = FALSE;
+  g_thread_join (query_thread);
+
+  gst_object_unref (pad);
+}
+
+GST_END_TEST;
+
 static Suite *
 gst_ghost_pad_suite (void)
 {
@@ -1351,6 +1396,7 @@ gst_ghost_pad_suite (void)
   tcase_add_test (tc_chain, test_activate_sink_and_src);
   tcase_add_test (tc_chain, test_activate_src_pull_mode);
   tcase_add_test (tc_chain, test_activate_sink_switch_mode);
+  tcase_add_test (tc_chain, test_stress_upstream_queries_while_tearing_down);
 
   return s;
 }