validate: Make Reporter.runner a MT safe weak reference

It can be used in any thread!
This commit is contained in:
Thibault Saunier 2017-06-01 16:38:25 -04:00
parent 4a38efb6a0
commit 9d3b760cf3
11 changed files with 59 additions and 41 deletions

View file

@ -270,18 +270,22 @@ gst_validate_bin_monitor_wrap_element (GstValidateBinMonitor * monitor,
GstElement * element) GstElement * element)
{ {
GstValidateElementMonitor *element_monitor; 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)); GST_DEBUG_OBJECT (monitor, "Wrapping element %s", GST_ELEMENT_NAME (element));
element_monitor = element_monitor =
GST_VALIDATE_ELEMENT_MONITOR_CAST (gst_validate_monitor_factory_create GST_VALIDATE_ELEMENT_MONITOR_CAST (gst_validate_monitor_factory_create
(GST_OBJECT_CAST (element), GST_VALIDATE_MONITOR_GET_RUNNER (monitor), (GST_OBJECT_CAST (element), runner, GST_VALIDATE_MONITOR_CAST (monitor)));
GST_VALIDATE_MONITOR_CAST (monitor)));
g_return_if_fail (element_monitor != NULL); g_return_if_fail (element_monitor != NULL);
GST_VALIDATE_MONITOR_LOCK (monitor); GST_VALIDATE_MONITOR_LOCK (monitor);
monitor->element_monitors = g_list_prepend (monitor->element_monitors, monitor->element_monitors = g_list_prepend (monitor->element_monitors,
element_monitor); element_monitor);
GST_VALIDATE_MONITOR_UNLOCK (monitor); GST_VALIDATE_MONITOR_UNLOCK (monitor);
gst_object_unref (runner);
} }
static void static void

View file

@ -315,17 +315,21 @@ gst_validate_element_monitor_wrap_pad (GstValidateElementMonitor * monitor,
GstPad * pad) GstPad * pad)
{ {
GstValidatePadMonitor *pad_monitor; 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)); GST_DEBUG_OBJECT (monitor, "Wrapping pad %s:%s", GST_DEBUG_PAD_NAME (pad));
pad_monitor = pad_monitor =
GST_VALIDATE_PAD_MONITOR (gst_validate_monitor_factory_create (GST_OBJECT GST_VALIDATE_PAD_MONITOR (gst_validate_monitor_factory_create (GST_OBJECT
(pad), GST_VALIDATE_MONITOR_GET_RUNNER (monitor), (pad), runner, GST_VALIDATE_MONITOR (monitor)));
GST_VALIDATE_MONITOR (monitor)));
g_return_if_fail (pad_monitor != NULL); g_return_if_fail (pad_monitor != NULL);
GST_VALIDATE_MONITOR_LOCK (monitor); GST_VALIDATE_MONITOR_LOCK (monitor);
monitor->pad_monitors = g_list_prepend (monitor->pad_monitors, pad_monitor); monitor->pad_monitors = g_list_prepend (monitor->pad_monitors, pad_monitor);
GST_VALIDATE_MONITOR_UNLOCK (monitor); GST_VALIDATE_MONITOR_UNLOCK (monitor);
gst_object_unref (runner);
} }
static void static void

View file

@ -271,6 +271,9 @@ _determine_reporting_level (GstValidateMonitor * monitor)
if (object) if (object)
gst_object_unref (object); gst_object_unref (object);
if (runner)
gst_object_unref (runner);
monitor->level = level; monitor->level = level;
} }
@ -341,6 +344,7 @@ gst_validate_monitor_attach_override (GstValidateMonitor * monitor,
GstValidateOverride * override) GstValidateOverride * override)
{ {
GstValidateRunner *runner; GstValidateRunner *runner;
GstValidateRunner *mrunner;
if (!gst_validate_override_can_attach (override, monitor)) { if (!gst_validate_override_can_attach (override, monitor)) {
GST_INFO_OBJECT (monitor, "Can not attach override %s", 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)); 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); GST_VALIDATE_MONITOR_OVERRIDES_LOCK (monitor);
if (runner) if (runner) {
g_assert (runner == g_assert (runner == mrunner);
gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER (monitor))); } else
else
gst_validate_reporter_set_runner (GST_VALIDATE_REPORTER (override), 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); g_queue_push_tail (&monitor->overrides, override);
GST_VALIDATE_MONITOR_OVERRIDES_UNLOCK (monitor); GST_VALIDATE_MONITOR_OVERRIDES_UNLOCK (monitor);
if (runner)
gst_object_unref (runner);
if (mrunner)
gst_object_unref (mrunner);
} }
static void 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)); g_value_take_object (value, gst_validate_monitor_get_pipeline (monitor));
break; break;
case PROP_RUNNER: 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; break;
case PROP_VALIDATE_PARENT: case PROP_VALIDATE_PARENT:
g_value_set_object (value, GST_VALIDATE_MONITOR_GET_PARENT (monitor)); g_value_set_object (value, GST_VALIDATE_MONITOR_GET_PARENT (monitor));

View file

@ -45,7 +45,6 @@ G_BEGIN_DECLS
#define GST_VALIDATE_MONITOR_CAST(obj) ((GstValidateMonitor*)(obj)) #define GST_VALIDATE_MONITOR_CAST(obj) ((GstValidateMonitor*)(obj))
#define GST_VALIDATE_MONITOR_CLASS_CAST(klass) ((GstValidateMonitorClass*)(klass)) #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_GET_PARENT(m) (GST_VALIDATE_MONITOR_CAST (m)->parent)
#define GST_VALIDATE_MONITOR_LOCK(m) \ #define GST_VALIDATE_MONITOR_LOCK(m) \

View file

@ -64,7 +64,7 @@ _get_property (GObject * object,
{ {
switch (property_id) { switch (property_id) {
case PROP_RUNNER: case PROP_RUNNER:
g_value_set_object (value, g_value_take_object (value,
gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER (object))); gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER (object)));
break; break;
default: default:

View file

@ -615,6 +615,9 @@ gst_validate_pipeline_monitor_create_scenarios (GstValidateBinMonitor * monitor)
gchar **scenarios = NULL; gchar **scenarios = NULL;
GstObject *target = GstObject *target =
gst_validate_monitor_get_target (GST_VALIDATE_MONITOR (monitor)); 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"))) { if ((scenarios_names = g_getenv ("GST_VALIDATE_SCENARIO"))) {
gint i; gint i;
@ -632,8 +635,8 @@ gst_validate_pipeline_monitor_create_scenarios (GstValidateBinMonitor * monitor)
} }
} }
monitor->scenario = monitor->scenario =
gst_validate_scenario_factory_create (GST_VALIDATE_MONITOR_GET_RUNNER gst_validate_scenario_factory_create (runner,
(monitor), GST_ELEMENT_CAST (target), scenario_v[0]); GST_ELEMENT_CAST (target), scenario_v[0]);
g_strfreev (scenario_v); g_strfreev (scenario_v);
} }
} }
@ -641,6 +644,8 @@ done:
g_strfreev (scenarios); g_strfreev (scenarios);
if (target) if (target)
gst_object_unref (target); gst_object_unref (target);
if (runner)
gst_object_unref (runner);
} }
/** /**

View file

@ -698,6 +698,7 @@ gst_validate_report_new (GstValidateIssue * issue,
issue_type_details = gst_validate_runner_get_reporting_level_for_name (runner, issue_type_details = gst_validate_runner_get_reporting_level_for_name (runner,
g_quark_to_string (issue->issue_id)); g_quark_to_string (issue->issue_id));
default_details = gst_validate_runner_get_default_reporting_details (runner); default_details = gst_validate_runner_get_default_reporting_details (runner);
gst_object_unref (runner);
if (reporter_details != GST_VALIDATE_SHOW_ALL && if (reporter_details != GST_VALIDATE_SHOW_ALL &&
reporter_details != GST_VALIDATE_SHOW_UNKNOWN) reporter_details != GST_VALIDATE_SHOW_UNKNOWN)
return report; return report;

View file

@ -37,7 +37,7 @@
typedef struct _GstValidateReporterPrivate typedef struct _GstValidateReporterPrivate
{ {
GstValidateRunner *runner; GWeakRef runner;
GHashTable *reports; GHashTable *reports;
char *name; char *name;
guint log_handler_id; guint log_handler_id;
@ -61,10 +61,6 @@ gst_validate_reporter_default_init (GstValidateReporterInterface * iface)
static void static void
_free_priv (GstValidateReporterPrivate * priv) _free_priv (GstValidateReporterPrivate * priv)
{ {
if (priv->runner)
g_object_remove_weak_pointer (G_OBJECT (priv->runner),
(gpointer) & priv->runner);
if (g_log_handler == priv) { if (g_log_handler == priv) {
g_log_set_default_handler (g_log_default_handler, NULL); g_log_set_default_handler (g_log_default_handler, NULL);
g_log_handler = NULL; g_log_handler = NULL;
@ -73,6 +69,7 @@ _free_priv (GstValidateReporterPrivate * priv)
g_hash_table_unref (priv->reports); g_hash_table_unref (priv->reports);
g_free (priv->name); g_free (priv->name);
g_mutex_clear (&priv->reports_lock); g_mutex_clear (&priv->reports_lock);
g_weak_ref_clear (&priv->runner);
g_slice_free (GstValidateReporterPrivate, priv); g_slice_free (GstValidateReporterPrivate, priv);
} }
@ -177,6 +174,7 @@ gst_validate_report_valist (GstValidateReporter * reporter,
GstValidateIssue *issue; GstValidateIssue *issue;
GstValidateReporterPrivate *priv = gst_validate_reporter_get_priv (reporter); GstValidateReporterPrivate *priv = gst_validate_reporter_get_priv (reporter);
GstValidateInterceptionReturn int_ret; GstValidateInterceptionReturn int_ret;
GstValidateRunner *runner = NULL;
issue = gst_validate_issue_from_id (issue_id); 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); prev_report = g_hash_table_lookup (priv->reports, (gconstpointer) issue_id);
runner = gst_validate_reporter_get_runner (reporter);
if (prev_report) { if (prev_report) {
GstValidateReportingDetails reporter_level = GstValidateReportingDetails reporter_level =
gst_validate_reporter_get_reporting_level (reporter); gst_validate_reporter_get_reporting_level (reporter);
GstValidateReportingDetails runner_level = GST_VALIDATE_SHOW_UNKNOWN; GstValidateReportingDetails runner_level = GST_VALIDATE_SHOW_UNKNOWN;
if (priv->runner) if (runner)
runner_level = runner_level = gst_validate_runner_get_default_reporting_level (runner);
gst_validate_runner_get_default_reporting_level (priv->runner);
if (reporter_level == GST_VALIDATE_SHOW_ALL || if (reporter_level == GST_VALIDATE_SHOW_ALL ||
(runner_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); g_hash_table_insert (priv->reports, (gpointer) issue_id, report);
GST_VALIDATE_REPORTER_REPORTS_UNLOCK (reporter); GST_VALIDATE_REPORTER_REPORTS_UNLOCK (reporter);
if (priv->runner && int_ret == GST_VALIDATE_REPORTER_REPORT) { if (runner && int_ret == GST_VALIDATE_REPORTER_REPORT) {
gst_validate_runner_add_report (priv->runner, report); gst_validate_runner_add_report (runner, report);
} }
if (gst_validate_report_check_abort (report)) { if (gst_validate_report_check_abort (report)) {
if (priv->runner) if (runner)
gst_validate_runner_printf (priv->runner); gst_validate_runner_printf (runner);
g_error ("Fatal report received: %" GST_VALIDATE_ERROR_REPORT_PRINT_FORMAT, g_error ("Fatal report received: %" GST_VALIDATE_ERROR_REPORT_PRINT_FORMAT,
GST_VALIDATE_REPORT_PRINT_ARGS (report)); GST_VALIDATE_REPORT_PRINT_ARGS (report));
} }
done: done:
if (runner)
gst_object_unref (runner);
g_free (message); g_free (message);
} }
@ -337,14 +338,14 @@ gst_validate_reporter_get_name (GstValidateReporter * reporter)
* gst_validate_reporter_get_runner: * gst_validate_reporter_get_runner:
* @reporter: The reporter to get the runner from * @reporter: The reporter to get the runner from
* *
* Returns: (transfer none): The runner * Returns: (transfer full): The runner
*/ */
GstValidateRunner * GstValidateRunner *
gst_validate_reporter_get_runner (GstValidateReporter * reporter) gst_validate_reporter_get_runner (GstValidateReporter * reporter)
{ {
GstValidateReporterPrivate *priv = gst_validate_reporter_get_priv (reporter); GstValidateReporterPrivate *priv = gst_validate_reporter_get_priv (reporter);
return priv->runner; return g_weak_ref_get (&priv->runner);
} }
void void
@ -353,14 +354,7 @@ gst_validate_reporter_set_runner (GstValidateReporter * reporter,
{ {
GstValidateReporterPrivate *priv = gst_validate_reporter_get_priv (reporter); GstValidateReporterPrivate *priv = gst_validate_reporter_get_priv (reporter);
priv->runner = runner; g_weak_ref_set (&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_object_notify (G_OBJECT (reporter), "validate-runner"); g_object_notify (G_OBJECT (reporter), "validate-runner");
} }

View file

@ -3008,7 +3008,7 @@ gst_validate_scenario_get_property (GObject * object, guint prop_id,
case PROP_RUNNER: case PROP_RUNNER:
/* we assume the runner is valid as long as this scenario is, /* we assume the runner is valid as long as this scenario is,
* no ref taken */ * no ref taken */
g_value_set_object (value, g_value_take_object (value,
gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER (object))); gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER (object)));
break; break;
case PROP_HANDLES_STATE: case PROP_HANDLES_STATE:

View file

@ -175,7 +175,7 @@ _get_property (GObject * object, guint prop_id,
case PROP_RUNNER: case PROP_RUNNER:
/* we assume the runner is valid as long as this scenario is, /* we assume the runner is valid as long as this scenario is,
* no ref taken */ * no ref taken */
g_value_set_object (value, g_value_take_object (value,
gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER (object))); gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER (object)));
break; break;
default: default:

View file

@ -267,11 +267,13 @@ static void
_runner_set (GObject * object, GParamSpec * pspec, gpointer user_data) _runner_set (GObject * object, GParamSpec * pspec, gpointer user_data)
{ {
ValidateSsimOverride *self = VALIDATE_SSIM_OVERRIDE (object); ValidateSsimOverride *self = VALIDATE_SSIM_OVERRIDE (object);
GstValidateRunner *runner =
gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER (self));
self->priv->is_attached = TRUE; self->priv->is_attached = TRUE;
g_signal_connect (gst_validate_reporter_get_runner (GST_VALIDATE_REPORTER g_signal_connect (runner, "stopping", G_CALLBACK (runner_stopping), self);
(self)), "stopping", G_CALLBACK (runner_stopping), self); gst_object_unref (runner);
} }
static ValidateSsimOverride * static ValidateSsimOverride *