From 7117e3e3df858812a3e9993dad35ca9699795702 Mon Sep 17 00:00:00 2001
From: Guillaume Desmottes <guillaume.desmottes@collabora.co.uk>
Date: Tue, 31 May 2016 12:32:16 +0200
Subject: [PATCH] validate: reporter: break cyclic references with reports
My patch fixing monitor leak (15e7f1bbfd84ce2cc5e6420fee2255c2be95e0f6)
introduced a ref cycle between GstValidateReporter and
GstValidateReport.
The reports uses its reporter so it needs a ref on it
to ensure it's stay alive. But reports are owned by
GstValidateReporter and/or GstValidateRunner.
Fix this by not taking a reference on the reporter but instead caching
its name.
Reviewed-by: Thibault Saunier <tsaunier@gnome.org>
Differential Revision: https://phabricator.freedesktop.org/D1029
---
validate/gst/validate/gst-validate-report.c | 16 +++++++++++-----
validate/gst/validate/gst-validate-report.h | 1 +
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/validate/gst/validate/gst-validate-report.c b/validate/gst/validate/gst-validate-report.c
index 3d535f1965..264409304d 100644
--- a/validate/gst/validate/gst-validate-report.c
+++ b/validate/gst/validate/gst-validate-report.c
@@ -508,8 +508,8 @@ gst_validate_report_get_issue_id (GstValidateReport * report)
static void
_report_free (GstValidateReport * report)
{
- g_object_unref (report->reporter);
g_free (report->message);
+ g_free (report->reporter_name);
g_list_free_full (report->shadow_reports,
(GDestroyNotify) gst_validate_report_unref);
g_list_free_full (report->repeated_reports,
@@ -529,7 +529,14 @@ gst_validate_report_new (GstValidateIssue * issue,
(GstMiniObjectFreeFunction) _report_free);
report->issue = issue;
- report->reporter = g_object_ref (reporter);
+ /* The reporter is owning a ref on the report so it doesn't keep a ref to
+ * avoid reference cycles. But the report can also be used by
+ * GstValidateRunner *after* that the reporter has been destroyed, so we
+ * cache the reporter name to avoid crashing in
+ * gst_validate_report_print_detected_on if the reporter has been destroyed.
+ */
+ report->reporter = reporter;
+ report->reporter_name = g_strdup (gst_validate_reporter_get_name (reporter));
report->message = g_strdup (message);
g_mutex_init (&report->shadow_reports_lock);
report->timestamp =
@@ -849,11 +856,10 @@ gst_validate_report_print_detected_on (GstValidateReport * report)
GList *tmp;
gst_validate_printf (NULL, "%*s Detected on <%s",
- 12, "", gst_validate_reporter_get_name (report->reporter));
+ 12, "", report->reporter_name);
for (tmp = report->shadow_reports; tmp; tmp = tmp->next) {
GstValidateReport *shadow_report = (GstValidateReport *) tmp->data;
- gst_validate_printf (NULL, ", %s",
- gst_validate_reporter_get_name (shadow_report->reporter));
+ gst_validate_printf (NULL, ", %s", shadow_report->reporter_name);
}
gst_validate_printf (NULL, ">\n");
}
diff --git a/validate/gst/validate/gst-validate-report.h b/validate/gst/validate/gst-validate-report.h
index 98499e3198..2617024f95 100644
--- a/validate/gst/validate/gst-validate-report.h
+++ b/validate/gst/validate/gst-validate-report.h
@@ -180,6 +180,7 @@ struct _GstValidateReport {
GList *repeated_reports;
GstValidateReportingDetails reporting_level;
+ gchar *reporter_name;
gpointer _gst_reserved[GST_PADDING];
};