Skip to content
Snippets Groups Projects
Commit 7117e3e3 authored by Guillaume Desmottes's avatar Guillaume Desmottes :goat: Committed by Thibault Saunier
Browse files

validate: reporter: break cyclic references with reports


My patch fixing monitor leak (15e7f1bb)
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: default avatarThibault Saunier <tsaunier@gnome.org>
Differential Revision: https://phabricator.freedesktop.org/D1029
parent 8708215c
No related branches found
No related tags found
Loading
...@@ -508,8 +508,8 @@ gst_validate_report_get_issue_id (GstValidateReport * report) ...@@ -508,8 +508,8 @@ gst_validate_report_get_issue_id (GstValidateReport * report)
static void static void
_report_free (GstValidateReport * report) _report_free (GstValidateReport * report)
{ {
g_object_unref (report->reporter);
g_free (report->message); g_free (report->message);
g_free (report->reporter_name);
g_list_free_full (report->shadow_reports, g_list_free_full (report->shadow_reports,
(GDestroyNotify) gst_validate_report_unref); (GDestroyNotify) gst_validate_report_unref);
g_list_free_full (report->repeated_reports, g_list_free_full (report->repeated_reports,
...@@ -529,7 +529,14 @@ gst_validate_report_new (GstValidateIssue * issue, ...@@ -529,7 +529,14 @@ gst_validate_report_new (GstValidateIssue * issue,
(GstMiniObjectFreeFunction) _report_free); (GstMiniObjectFreeFunction) _report_free);
report->issue = issue; 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); report->message = g_strdup (message);
g_mutex_init (&report->shadow_reports_lock); g_mutex_init (&report->shadow_reports_lock);
report->timestamp = report->timestamp =
...@@ -849,11 +856,10 @@ gst_validate_report_print_detected_on (GstValidateReport * report) ...@@ -849,11 +856,10 @@ gst_validate_report_print_detected_on (GstValidateReport * report)
GList *tmp; GList *tmp;
gst_validate_printf (NULL, "%*s Detected on <%s", 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) { for (tmp = report->shadow_reports; tmp; tmp = tmp->next) {
GstValidateReport *shadow_report = (GstValidateReport *) tmp->data; GstValidateReport *shadow_report = (GstValidateReport *) tmp->data;
gst_validate_printf (NULL, ", %s", gst_validate_printf (NULL, ", %s", shadow_report->reporter_name);
gst_validate_reporter_get_name (shadow_report->reporter));
} }
gst_validate_printf (NULL, ">\n"); gst_validate_printf (NULL, ">\n");
} }
......
...@@ -180,6 +180,7 @@ struct _GstValidateReport { ...@@ -180,6 +180,7 @@ struct _GstValidateReport {
GList *repeated_reports; GList *repeated_reports;
GstValidateReportingDetails reporting_level; GstValidateReportingDetails reporting_level;
gchar *reporter_name;
gpointer _gst_reserved[GST_PADDING]; gpointer _gst_reserved[GST_PADDING];
}; };
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment