harness: fixed race condition on forward pad while forwarding sticky events to sink...

Co-authored-by: Camilo Celis <camilo@pexip.com>
Co-authored-by: Havard Graff <hgr@pexip.com>

diff --git a/libs/gst/check/gstharness.c b/libs/gst/check/gstharness.c
index f82a879..d6c26f6 100644 (file)
--- a/libs/gst/check/gstharness.c
+++ b/libs/gst/check/gstharness.c
@@ -1556,13 +1556,14 @@ gst_harness_set_forwarding (GstHarness * h, gboolean forwarding)
     gst_harness_set_forwarding (h->sink_harness, forwarding);
 }
 
+/*
+* Call with HARNESS_LOCK
+*/
 static void
 gst_harness_set_forward_pad (GstHarness * h, GstPad * fwdpad)
 {
-  HARNESS_LOCK (h);
   gst_object_replace ((GstObject **) & h->priv->sink_forward_pad,
       (GstObject *) fwdpad);
-  HARNESS_UNLOCK (h);
 }
 
 /**
@@ -2308,7 +2309,11 @@ gst_harness_add_src_harness (GstHarness * h,
   if (h->src_harness)
     gst_harness_teardown (h->src_harness);
   h->src_harness = src_harness;
+
+  HARNESS_LOCK (h->src_harness);
   gst_harness_set_forward_pad (h->src_harness, h->srcpad);
+  HARNESS_UNLOCK (h->src_harness);
+
   h->src_harness->priv->has_clock_wait = has_clock_wait;
   gst_harness_set_forwarding (h->src_harness, h->priv->forwarding);
 }
@@ -2494,24 +2499,35 @@ forward_sticky_events (GstPad * pad, GstEvent ** ev, gpointer user_data)
 void
 gst_harness_add_sink_harness (GstHarness * h, GstHarness * sink_harness)
 {
-  GstHarnessPrivate *priv = h->priv;
+  GstHarnessPrivate *priv;
+  GstPad *fwdpad;
+
+  HARNESS_LOCK (h);
+  priv = h->priv;
 
   if (h->sink_harness) {
     gst_harness_set_forward_pad (h, NULL);
     gst_harness_teardown (h->sink_harness);
   }
   h->sink_harness = sink_harness;
-  gst_harness_set_forward_pad (h, h->sink_harness->srcpad);
-  HARNESS_LOCK (h);
-  if (priv->forwarding && h->sinkpad && h->priv->sink_forward_pad) {
-    GstPad *fwdpad = gst_object_ref (h->priv->sink_forward_pad);
+
+  fwdpad = h->sink_harness->srcpad;
+  if (fwdpad)
+    gst_object_ref (fwdpad);
+
+  if (priv->forwarding && h->sinkpad && fwdpad) {
     HARNESS_UNLOCK (h);
     gst_pad_sticky_events_foreach (h->sinkpad, forward_sticky_events, fwdpad);
-    gst_object_unref (fwdpad);
-  } else {
-    HARNESS_UNLOCK (h);
+    HARNESS_LOCK (h);
   }
+
+  gst_harness_set_forward_pad (h, fwdpad);
+  if (fwdpad)
+    gst_object_unref (fwdpad);
+
   gst_harness_set_forwarding (h->sink_harness, priv->forwarding);
+
+  HARNESS_UNLOCK (h);
 }
 
 /**

diff --git a/tests/check/libs/gstharness.c b/tests/check/libs/gstharness.c
index 633d538..01cf841 100644 (file)
--- a/tests/check/libs/gstharness.c
+++ b/tests/check/libs/gstharness.c
@@ -170,6 +170,45 @@ GST_START_TEST (test_forward_event_and_query_to_sink_harness_while_teardown)
 
 GST_END_TEST;
 
+static void
+push_sticky_events (gpointer data, G_GNUC_UNUSED gpointer user_data)
+{
+  GstHarness *h;
+  GstCaps *caps;
+  GstSegment segment;
+
+  h = (GstHarness *) user_data;
+
+  gst_harness_push_event (h, gst_event_new_stream_start ("999"));
+
+  caps = gst_caps_new_empty_simple ("mycaps");
+  gst_harness_push_event (h, gst_event_new_caps (caps));
+  gst_caps_unref (caps);
+
+  gst_segment_init (&segment, GST_FORMAT_TIME);
+  gst_harness_push_event (h, gst_event_new_segment (&segment));
+}
+
+GST_START_TEST (test_forward_sticky_events_to_sink_harness_while_teardown)
+{
+  GstHarness *h = gst_harness_new ("identity");
+  GstHarnessThread *e_thread = gst_harness_stress_custom_start (h, NULL,
+      push_sticky_events, h, 0);
+  gdouble duration = 1.0;
+  GTimer *timer = g_timer_new ();
+
+  while (g_timer_elapsed (timer, NULL) < duration) {
+    gst_harness_add_sink (h, "fakesink");
+    g_thread_yield ();
+  }
+
+  g_timer_destroy (timer);
+  gst_harness_stress_thread_stop (e_thread);
+  gst_harness_teardown (h);
+}
+
+GST_END_TEST;
+
 static GstHarness *
 harness_new_and_fill_with_data (void)
 {
@@ -261,6 +300,9 @@ gst_harness_suite (void)
 
   tcase_add_test (tc_chain,
       test_forward_event_and_query_to_sink_harness_while_teardown);
+  tcase_add_test (tc_chain,
+      test_forward_sticky_events_to_sink_harness_while_teardown);
+
   tcase_add_test (tc_chain, test_get_all_data);
 
   return s;