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

diff --git a/validate/gst/validate/gst-validate-report.c b/validate/gst/validate/gst-validate-report.c
index 3d535f1..2644093 100644 (file)
--- 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 98499e3..2617024 100644 (file)
--- 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];
 };