From: Thibault Saunier <thibault.saunier@osg.samsung.com>
Date: Thu, 1 Jun 2017 20:38:25 +0000 (-0400)
Subject: validate: Make Reporter.runner a MT safe weak reference
X-Git-Tag: 1.19.3~491^2~704
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=9d3b760cf39fd0ef6087c03140bd567a21aebb0a;p=platform%2Fupstream%2Fgstreamer.git

validate: Make Reporter.runner a MT safe weak reference

It can be used in any thread!
---

diff --git a/validate/gst/validate/gst-validate-bin-monitor.c b/validate/gst/validate/gst-validate-bin-monitor.c
index ec692a6..6f20fa2 100644
--- a/validate/gst/validate/gst-validate-bin-monitor.c
+++ b/validate/gst/validate/gst-validate-bin-monitor.c
@@ -270,18 +270,22 @@ gst_validate_bin_monitor_wrap_element (GstValidateBinMonitor * monitor,
     GstElement * element)
 {
   GstValidateElementMonitor *element_monitor;
+  GstValidateRunner *runner =
+      gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER (monitor));
+
   GST_DEBUG_OBJECT (monitor, "Wrapping element %s", GST_ELEMENT_NAME (element));
 
   element_monitor =
       GST_VALIDATE_ELEMENT_MONITOR_CAST (gst_validate_monitor_factory_create
-      (GST_OBJECT_CAST (element), GST_VALIDATE_MONITOR_GET_RUNNER (monitor),
-          GST_VALIDATE_MONITOR_CAST (monitor)));
+      (GST_OBJECT_CAST (element), runner, GST_VALIDATE_MONITOR_CAST (monitor)));
   g_return_if_fail (element_monitor != NULL);
 
   GST_VALIDATE_MONITOR_LOCK (monitor);
   monitor->element_monitors = g_list_prepend (monitor->element_monitors,
       element_monitor);
   GST_VALIDATE_MONITOR_UNLOCK (monitor);
+
+  gst_object_unref (runner);
 }
 
 static void
diff --git a/validate/gst/validate/gst-validate-element-monitor.c b/validate/gst/validate/gst-validate-element-monitor.c
index 9019a18..01332a1 100644
--- a/validate/gst/validate/gst-validate-element-monitor.c
+++ b/validate/gst/validate/gst-validate-element-monitor.c
@@ -315,17 +315,21 @@ gst_validate_element_monitor_wrap_pad (GstValidateElementMonitor * monitor,
     GstPad * pad)
 {
   GstValidatePadMonitor *pad_monitor;
+  GstValidateRunner *runner =
+      gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER (monitor));
+
   GST_DEBUG_OBJECT (monitor, "Wrapping pad %s:%s", GST_DEBUG_PAD_NAME (pad));
 
   pad_monitor =
       GST_VALIDATE_PAD_MONITOR (gst_validate_monitor_factory_create (GST_OBJECT
-          (pad), GST_VALIDATE_MONITOR_GET_RUNNER (monitor),
-          GST_VALIDATE_MONITOR (monitor)));
+          (pad), runner, GST_VALIDATE_MONITOR (monitor)));
   g_return_if_fail (pad_monitor != NULL);
 
   GST_VALIDATE_MONITOR_LOCK (monitor);
   monitor->pad_monitors = g_list_prepend (monitor->pad_monitors, pad_monitor);
   GST_VALIDATE_MONITOR_UNLOCK (monitor);
+
+  gst_object_unref (runner);
 }
 
 static void
diff --git a/validate/gst/validate/gst-validate-monitor.c b/validate/gst/validate/gst-validate-monitor.c
index 41bdb16..925eded 100644
--- a/validate/gst/validate/gst-validate-monitor.c
+++ b/validate/gst/validate/gst-validate-monitor.c
@@ -271,6 +271,9 @@ _determine_reporting_level (GstValidateMonitor * monitor)
   if (object)
     gst_object_unref (object);
 
+  if (runner)
+    gst_object_unref (runner);
+
   monitor->level = level;
 }
 
@@ -341,6 +344,7 @@ gst_validate_monitor_attach_override (GstValidateMonitor * monitor,
     GstValidateOverride * override)
 {
   GstValidateRunner *runner;
+  GstValidateRunner *mrunner;
 
   if (!gst_validate_override_can_attach (override, monitor)) {
     GST_INFO_OBJECT (monitor, "Can not attach override %s",
@@ -350,16 +354,20 @@ gst_validate_monitor_attach_override (GstValidateMonitor * monitor,
   }
 
   runner = gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER (override));
-
+  mrunner = gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER (monitor));
   GST_VALIDATE_MONITOR_OVERRIDES_LOCK (monitor);
-  if (runner)
-    g_assert (runner ==
-        gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER (monitor)));
-  else
+  if (runner) {
+    g_assert (runner == mrunner);
+  } else
     gst_validate_reporter_set_runner (GST_VALIDATE_REPORTER (override),
-        gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER (monitor)));
+        mrunner);
   g_queue_push_tail (&monitor->overrides, override);
   GST_VALIDATE_MONITOR_OVERRIDES_UNLOCK (monitor);
+
+  if (runner)
+    gst_object_unref (runner);
+  if (mrunner)
+    gst_object_unref (mrunner);
 }
 
 static void
@@ -417,7 +425,8 @@ gst_validate_monitor_get_property (GObject * object, guint prop_id,
       g_value_take_object (value, gst_validate_monitor_get_pipeline (monitor));
       break;
     case PROP_RUNNER:
-      g_value_set_object (value, GST_VALIDATE_MONITOR_GET_RUNNER (monitor));
+      g_value_take_object (value,
+          gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER (monitor)));
       break;
     case PROP_VALIDATE_PARENT:
       g_value_set_object (value, GST_VALIDATE_MONITOR_GET_PARENT (monitor));
diff --git a/validate/gst/validate/gst-validate-monitor.h b/validate/gst/validate/gst-validate-monitor.h
index 66f04c0..252640d 100644
--- a/validate/gst/validate/gst-validate-monitor.h
+++ b/validate/gst/validate/gst-validate-monitor.h
@@ -45,7 +45,6 @@ G_BEGIN_DECLS
 #define GST_VALIDATE_MONITOR_CAST(obj)                ((GstValidateMonitor*)(obj))
 #define GST_VALIDATE_MONITOR_CLASS_CAST(klass)        ((GstValidateMonitorClass*)(klass))
 
-#define GST_VALIDATE_MONITOR_GET_RUNNER(m) (gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER_CAST (m)))
 #define GST_VALIDATE_MONITOR_GET_PARENT(m) (GST_VALIDATE_MONITOR_CAST (m)->parent)
 
 #define GST_VALIDATE_MONITOR_LOCK(m)			\
diff --git a/validate/gst/validate/gst-validate-override.c b/validate/gst/validate/gst-validate-override.c
index 039fb63..c2ff1f1 100644
--- a/validate/gst/validate/gst-validate-override.c
+++ b/validate/gst/validate/gst-validate-override.c
@@ -64,7 +64,7 @@ _get_property (GObject * object,
 {
   switch (property_id) {
     case PROP_RUNNER:
-      g_value_set_object (value,
+      g_value_take_object (value,
           gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER (object)));
       break;
     default:
diff --git a/validate/gst/validate/gst-validate-pipeline-monitor.c b/validate/gst/validate/gst-validate-pipeline-monitor.c
index 7e25b2d..58c062b 100644
--- a/validate/gst/validate/gst-validate-pipeline-monitor.c
+++ b/validate/gst/validate/gst-validate-pipeline-monitor.c
@@ -615,6 +615,9 @@ gst_validate_pipeline_monitor_create_scenarios (GstValidateBinMonitor * monitor)
   gchar **scenarios = NULL;
   GstObject *target =
       gst_validate_monitor_get_target (GST_VALIDATE_MONITOR (monitor));
+  GstValidateRunner *runner =
+      gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER (monitor));
+
 
   if ((scenarios_names = g_getenv ("GST_VALIDATE_SCENARIO"))) {
     gint i;
@@ -632,8 +635,8 @@ gst_validate_pipeline_monitor_create_scenarios (GstValidateBinMonitor * monitor)
         }
       }
       monitor->scenario =
-          gst_validate_scenario_factory_create (GST_VALIDATE_MONITOR_GET_RUNNER
-          (monitor), GST_ELEMENT_CAST (target), scenario_v[0]);
+          gst_validate_scenario_factory_create (runner,
+          GST_ELEMENT_CAST (target), scenario_v[0]);
       g_strfreev (scenario_v);
     }
   }
@@ -641,6 +644,8 @@ done:
   g_strfreev (scenarios);
   if (target)
     gst_object_unref (target);
+  if (runner)
+    gst_object_unref (runner);
 }
 
 /**
diff --git a/validate/gst/validate/gst-validate-report.c b/validate/gst/validate/gst-validate-report.c
index 017a3c6..1b98586 100644
--- a/validate/gst/validate/gst-validate-report.c
+++ b/validate/gst/validate/gst-validate-report.c
@@ -698,6 +698,7 @@ gst_validate_report_new (GstValidateIssue * issue,
   issue_type_details = gst_validate_runner_get_reporting_level_for_name (runner,
       g_quark_to_string (issue->issue_id));
   default_details = gst_validate_runner_get_default_reporting_details (runner);
+  gst_object_unref (runner);
   if (reporter_details != GST_VALIDATE_SHOW_ALL &&
       reporter_details != GST_VALIDATE_SHOW_UNKNOWN)
     return report;
diff --git a/validate/gst/validate/gst-validate-reporter.c b/validate/gst/validate/gst-validate-reporter.c
index 4b67ade..de7f096 100644
--- a/validate/gst/validate/gst-validate-reporter.c
+++ b/validate/gst/validate/gst-validate-reporter.c
@@ -37,7 +37,7 @@
 
 typedef struct _GstValidateReporterPrivate
 {
-  GstValidateRunner *runner;
+  GWeakRef runner;
   GHashTable *reports;
   char *name;
   guint log_handler_id;
@@ -61,10 +61,6 @@ gst_validate_reporter_default_init (GstValidateReporterInterface * iface)
 static void
 _free_priv (GstValidateReporterPrivate * priv)
 {
-  if (priv->runner)
-    g_object_remove_weak_pointer (G_OBJECT (priv->runner),
-        (gpointer) & priv->runner);
-
   if (g_log_handler == priv) {
     g_log_set_default_handler (g_log_default_handler, NULL);
     g_log_handler = NULL;
@@ -73,6 +69,7 @@ _free_priv (GstValidateReporterPrivate * priv)
   g_hash_table_unref (priv->reports);
   g_free (priv->name);
   g_mutex_clear (&priv->reports_lock);
+  g_weak_ref_clear (&priv->runner);
   g_slice_free (GstValidateReporterPrivate, priv);
 }
 
@@ -177,6 +174,7 @@ gst_validate_report_valist (GstValidateReporter * reporter,
   GstValidateIssue *issue;
   GstValidateReporterPrivate *priv = gst_validate_reporter_get_priv (reporter);
   GstValidateInterceptionReturn int_ret;
+  GstValidateRunner *runner = NULL;
 
   issue = gst_validate_issue_from_id (issue_id);
 
@@ -216,14 +214,14 @@ gst_validate_report_valist (GstValidateReporter * reporter,
 
   prev_report = g_hash_table_lookup (priv->reports, (gconstpointer) issue_id);
 
+  runner = gst_validate_reporter_get_runner (reporter);
   if (prev_report) {
     GstValidateReportingDetails reporter_level =
         gst_validate_reporter_get_reporting_level (reporter);
     GstValidateReportingDetails runner_level = GST_VALIDATE_SHOW_UNKNOWN;
 
-    if (priv->runner)
-      runner_level =
-          gst_validate_runner_get_default_reporting_level (priv->runner);
+    if (runner)
+      runner_level = gst_validate_runner_get_default_reporting_level (runner);
 
     if (reporter_level == GST_VALIDATE_SHOW_ALL ||
         (runner_level == GST_VALIDATE_SHOW_ALL &&
@@ -238,19 +236,22 @@ gst_validate_report_valist (GstValidateReporter * reporter,
   g_hash_table_insert (priv->reports, (gpointer) issue_id, report);
   GST_VALIDATE_REPORTER_REPORTS_UNLOCK (reporter);
 
-  if (priv->runner && int_ret == GST_VALIDATE_REPORTER_REPORT) {
-    gst_validate_runner_add_report (priv->runner, report);
+  if (runner && int_ret == GST_VALIDATE_REPORTER_REPORT) {
+    gst_validate_runner_add_report (runner, report);
   }
 
   if (gst_validate_report_check_abort (report)) {
-    if (priv->runner)
-      gst_validate_runner_printf (priv->runner);
+    if (runner)
+      gst_validate_runner_printf (runner);
 
     g_error ("Fatal report received: %" GST_VALIDATE_ERROR_REPORT_PRINT_FORMAT,
         GST_VALIDATE_REPORT_PRINT_ARGS (report));
   }
 
 done:
+  if (runner)
+    gst_object_unref (runner);
+
   g_free (message);
 }
 
@@ -337,14 +338,14 @@ gst_validate_reporter_get_name (GstValidateReporter * reporter)
  * gst_validate_reporter_get_runner:
  * @reporter: The reporter to get the runner from
  *
- * Returns: (transfer none): The runner
+ * Returns: (transfer full): The runner
  */
 GstValidateRunner *
 gst_validate_reporter_get_runner (GstValidateReporter * reporter)
 {
   GstValidateReporterPrivate *priv = gst_validate_reporter_get_priv (reporter);
 
-  return priv->runner;
+  return g_weak_ref_get (&priv->runner);
 }
 
 void
@@ -353,14 +354,7 @@ gst_validate_reporter_set_runner (GstValidateReporter * reporter,
 {
   GstValidateReporterPrivate *priv = gst_validate_reporter_get_priv (reporter);
 
-  priv->runner = runner;
-
-  /* The runner is supposed to stay alive during the whole scenario but if
-   * we are using another tracer we may have messages catched after it has been
-   * destroyed. This may happen if the 'leaks' tracer detected leaks for
-   * example. */
-  if (runner)
-    g_object_add_weak_pointer (G_OBJECT (runner), (gpointer) & priv->runner);
+  g_weak_ref_set (&priv->runner, runner);
 
   g_object_notify (G_OBJECT (reporter), "validate-runner");
 }
diff --git a/validate/gst/validate/gst-validate-scenario.c b/validate/gst/validate/gst-validate-scenario.c
index 0526a92..9b5a86a 100644
--- a/validate/gst/validate/gst-validate-scenario.c
+++ b/validate/gst/validate/gst-validate-scenario.c
@@ -3008,7 +3008,7 @@ gst_validate_scenario_get_property (GObject * object, guint prop_id,
     case PROP_RUNNER:
       /* we assume the runner is valid as long as this scenario is,
        * no ref taken */
-      g_value_set_object (value,
+      g_value_take_object (value,
           gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER (object)));
       break;
     case PROP_HANDLES_STATE:
diff --git a/validate/gst/validate/media-descriptor.c b/validate/gst/validate/media-descriptor.c
index a48f9f0..42f8a0a 100644
--- a/validate/gst/validate/media-descriptor.c
+++ b/validate/gst/validate/media-descriptor.c
@@ -175,7 +175,7 @@ _get_property (GObject * object, guint prop_id,
     case PROP_RUNNER:
       /* we assume the runner is valid as long as this scenario is,
        * no ref taken */
-      g_value_set_object (value,
+      g_value_take_object (value,
           gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER (object)));
       break;
     default:
diff --git a/validate/plugins/ssim/gstvalidatessim.c b/validate/plugins/ssim/gstvalidatessim.c
index a5ca230..741e50a 100644
--- a/validate/plugins/ssim/gstvalidatessim.c
+++ b/validate/plugins/ssim/gstvalidatessim.c
@@ -267,11 +267,13 @@ static void
 _runner_set (GObject * object, GParamSpec * pspec, gpointer user_data)
 {
   ValidateSsimOverride *self = VALIDATE_SSIM_OVERRIDE (object);
+  GstValidateRunner *runner =
+      gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER (self));
 
   self->priv->is_attached = TRUE;
 
-  g_signal_connect (gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER
-          (self)), "stopping", G_CALLBACK (runner_stopping), self);
+  g_signal_connect (runner, "stopping", G_CALLBACK (runner_stopping), self);
+  gst_object_unref (runner);
 }
 
 static ValidateSsimOverride *