From 739144d0d576797397b56dcb25364ff8c2b33866 Mon Sep 17 00:00:00 2001
From: Stian Selnes <stian@pexip.com>
Date: Thu, 26 Feb 2015 14:35:35 +0100
Subject: [PATCH] gstinfo: Make GST_DEBUG_PAD_NAME "MT crash safe" (for gcc)

The pad may be unparented while this macro is called which could result
in a segfault. The new macro protects against this. The parent may still
be disposed while the macro is called, but this will not result in a
crash (but the parent name may be garbage). Using gst_pad_get_parent ()
is undesirable since it takes the object lock.

The patch take advantage of compound expressions available as a C
extension in GCC and some other compilers.
---
 gst/gstinfo.h             | 25 +++++++++++++---
 tests/check/gst/gstinfo.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+), 4 deletions(-)

diff --git a/gst/gstinfo.h b/gst/gstinfo.h
index 6ef058d..88a00ff 100644
--- a/gst/gstinfo.h
+++ b/gst/gstinfo.h
@@ -223,6 +223,26 @@ struct _GstDebugCategory {
  */
 #define GST_STR_NULL(str) ((str) ? (str) : "(NULL)")
 
+#if defined (__GNUC__)
+#define _GST_OBJECT_PARENT_NAME(obj)                                  \
+  ({                                                                  \
+    GstObject * parent;                                               \
+    const gchar * parent_name;                                        \
+    if (G_LIKELY ((parent = GST_OBJECT_PARENT (obj)) != NULL)) {      \
+      /* parent may be disposed, but accessing name will not crash*/  \
+      parent_name = GST_OBJECT_NAME (parent);                         \
+    } else {                                                          \
+      parent_name = "''";                                             \
+    }                                                                 \
+    parent_name;                                                      \
+  })
+#else
+/* Not MT safe since object may be unparented after check */
+#define _GST_OBJECT_PARENT_NAME(obj)                                  \
+  (GST_OBJECT_PARENT (obj) != NULL) ?                                 \
+  GST_OBJECT_NAME (GST_OBJECT_PARENT (obj)) : "''"
+#endif
+
 /* FIXME, not MT safe */
 /**
  * GST_DEBUG_PAD_NAME:
@@ -232,10 +252,7 @@ struct _GstDebugCategory {
  * statements.
  */
 #define GST_DEBUG_PAD_NAME(pad) \
-  (pad != NULL) ?  \
-  ((GST_OBJECT_PARENT(pad) != NULL) ? \
-  GST_STR_NULL (GST_OBJECT_NAME (GST_OBJECT_PARENT(pad))) : \
-  "''" ) : "''", \
+  (pad != NULL) ? GST_STR_NULL (_GST_OBJECT_PARENT_NAME (pad)) : "''",  \
   (pad != NULL) ? GST_STR_NULL (GST_OBJECT_NAME (pad)) : "''"
 
 /**
diff --git a/tests/check/gst/gstinfo.c b/tests/check/gst/gstinfo.c
index 763a223..edab3d7 100644
--- a/tests/check/gst/gstinfo.c
+++ b/tests/check/gst/gstinfo.c
@@ -422,6 +422,73 @@ GST_START_TEST (info_fourcc)
 
 GST_END_TEST;
 
+typedef struct
+{
+  gboolean thread_running;
+  gpointer user_data;
+} StressTestData;
+
+static gpointer
+debug_pad_name_func (gpointer user_data)
+{
+  StressTestData *data = user_data;
+  GstPad *pad = data->user_data;
+
+  while (data->thread_running) {
+    gchar *name = g_strdup_printf ("%s:%s", GST_DEBUG_PAD_NAME (pad));
+    fail_unless (name != NULL);
+    g_free (name);
+  }
+
+  return NULL;
+}
+
+static gpointer
+unparent_func (gpointer user_data)
+{
+  StressTestData *data = user_data;
+  GstObject *obj = data->user_data;
+
+  while (data->thread_running) {
+    GstObject *parent = gst_object_get_parent (obj);
+    gst_object_unparent (obj);
+    g_thread_yield ();
+    gst_object_set_parent (obj, parent);
+    gst_object_unref (parent);
+  }
+
+  return NULL;
+}
+
+GST_START_TEST(gst_debug_pad_name_stress)
+{
+  GThread *pad_name_thread, *unparent_thread;
+  GstPad *pad = gst_pad_new ("testpad", GST_PAD_UNKNOWN);
+  GstElement *element = gst_element_factory_make ("identity", NULL);
+  StressTestData data = { TRUE, pad };
+
+  g_object_ref (G_OBJECT (pad));
+  gst_object_set_parent (GST_OBJECT (pad), GST_OBJECT (element));
+
+  pad_name_thread = g_thread_new (
+      "pad_name_thread", (GThreadFunc) debug_pad_name_func, &data);
+  unparent_thread = g_thread_new (
+      "gst_object_unparent_thread", (GThreadFunc) unparent_func, &data);
+
+  /* test duration */
+  g_usleep (G_USEC_PER_SEC / 10);
+
+  data.thread_running = FALSE;
+  g_thread_join (pad_name_thread);
+  g_thread_join (unparent_thread);
+
+  gst_object_unparent (GST_OBJECT (pad));
+  gst_object_unref (pad);
+  gst_object_unref (element);
+}
+
+GST_END_TEST;
+
 static Suite *
 gst_info_suite (void)
 {
@@ -444,6 +511,12 @@ gst_info_suite (void)
   tcase_add_test (tc_chain, info_set_and_unset_multiple);
 #endif
 
+#if defined (__GNUC__)
+  tcase_add_test (tc_chain, gst_debug_pad_name_stress);
+#else
+  tcase_skip_broken_test (tc_chain, gst_debug_pad_name_stress);
+#endif
+
   return s;
 }