From ff9a78196c032588d75b99f6dfb74817bde196c2 Mon Sep 17 00:00:00 2001 From: Stian Selnes Date: Wed, 5 Aug 2015 10:07:50 +0200 Subject: [PATCH] harness: Fix race for gst_harness_element_ref In order for gst_harness_new_full to be MT-safe the increase and decrease of HARNESS_REF must be MT-safe. This allows for creating multiple harnesses from different threads wrapping the same element. https://bugzilla.gnome.org/show_bug.cgi?id=754661 --- libs/gst/check/gstharness.c | 17 ++++++++++++++--- tests/check/libs/gstharness.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/libs/gst/check/gstharness.c b/libs/gst/check/gstharness.c index cfaa5861a0..6a8cde3c84 100644 --- a/libs/gst/check/gstharness.c +++ b/libs/gst/check/gstharness.c @@ -433,7 +433,10 @@ gst_harness_src_query (GstPad * pad, GstObject * parent, GstQuery * query) static void gst_harness_element_ref (GstHarness * h) { - guint *data = g_object_get_data (G_OBJECT (h->element), HARNESS_REF); + guint *data; + + GST_OBJECT_LOCK (h->element); + data = g_object_get_data (G_OBJECT (h->element), HARNESS_REF); if (data == NULL) { data = g_new0 (guint, 1); *data = 1; @@ -441,15 +444,23 @@ gst_harness_element_ref (GstHarness * h) } else { (*data)++; } + GST_OBJECT_UNLOCK (h->element); } static guint gst_harness_element_unref (GstHarness * h) { - guint *data = g_object_get_data (G_OBJECT (h->element), HARNESS_REF); + guint *data; + guint ret; + + GST_OBJECT_LOCK (h->element); + data = g_object_get_data (G_OBJECT (h->element), HARNESS_REF); g_assert (data != NULL); (*data)--; - return *data; + ret = *data; + GST_OBJECT_UNLOCK (h->element); + + return ret; } static void diff --git a/tests/check/libs/gstharness.c b/tests/check/libs/gstharness.c index 67abab2aa2..ef55afc6e2 100644 --- a/tests/check/libs/gstharness.c +++ b/tests/check/libs/gstharness.c @@ -22,6 +22,33 @@ #include #include +static void +create_destroy_element_harness (gpointer data, gpointer user_data) +{ + GstElement * element = user_data; + GstHarness * h = gst_harness_new_with_element (element, NULL, NULL); + gst_harness_teardown (h); +} + +GST_START_TEST(test_harness_element_ref) +{ + GstHarness * h = gst_harness_new ("identity"); + GstHarnessThread * threads[100]; + gint i; + + for (i = 0; i < G_N_ELEMENTS (threads); i++) + threads[i] = gst_harness_stress_custom_start (h, NULL, + create_destroy_element_harness, h->element, 0); + for (i = 0; i < G_N_ELEMENTS (threads); i++) + gst_harness_stress_thread_stop (threads[i]); + + fail_unless_equals_int (G_OBJECT (h->element)->ref_count, 1); + + gst_harness_teardown (h); +} +GST_END_TEST; + + GST_START_TEST(test_src_harness) { GstHarness * h = gst_harness_new ("identity"); @@ -88,6 +115,7 @@ gst_harness_suite (void) suite_add_tcase (s, tc_chain); + tcase_add_test (tc_chain, test_harness_element_ref); tcase_add_test (tc_chain, test_src_harness); tcase_add_test (tc_chain, test_src_harness_no_forwarding);