From 88f4ade0722977ce30f139635b64826b0146e364 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 1 Mar 2020 20:41:25 +1100 Subject: [PATCH 1/3] ProblemReport: Clear logs iteratively, not recursively Clear logs iteratively to avoid recursively freeing all log messages on the stack at once. --- src/engine/api/geary-problem-report.vala | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/engine/api/geary-problem-report.vala b/src/engine/api/geary-problem-report.vala index 7a4b056d..5f21b856 100644 --- a/src/engine/api/geary-problem-report.vala +++ b/src/engine/api/geary-problem-report.vala @@ -30,6 +30,22 @@ public class Geary.ProblemReport : Object { this.latest_log = Logging.get_latest_record(); } + ~ProblemReport() { + // Manually clear each log record in a loop if we have the + // only reference to it so that finalisation of each is an + // iterative process. If we just nulled out the record, + // finalising the first would cause second to be finalised, + // which would finalise the third, etc., and the recursion + // could cause the stack to blow right out for large log + // buffers. + Logging.Record? earliest = this.earliest_log; + this.earliest_log = null; + this.latest_log = null; + while (earliest != null) { + earliest = earliest.next; + } + } + /** Returns a string representation of the report, for debugging only. */ public string to_string() { return "%s".printf( From b0be1a79b1e04b38f4f9eb448d3058a0d30cc202 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 1 Mar 2020 21:39:04 +1100 Subject: [PATCH 2/3] Geary.Logging.Record: Add copy constructor --- src/engine/api/geary-logging.vala | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/engine/api/geary-logging.vala b/src/engine/api/geary-logging.vala index 11f7b09d..dcdfd76c 100644 --- a/src/engine/api/geary-logging.vala +++ b/src/engine/api/geary-logging.vala @@ -208,6 +208,36 @@ public class Record { this.states.length = state_count; } + /** + * Copy constructor. + * + * Copies all properties of the given record except its next + * record. + */ + public Record.copy(Record other) { + this.domain = other.domain; + this.account = other.account; + this.service = other.service; + this.folder = other.folder; + this.flags = other.flags; + this.message = other.message; + this.source_filename = other.source_filename; + this.source_line_number = other.source_line_number; + this.source_function = other.source_function; + this.levels = other.levels; + this.timestamp = other.timestamp; + + // Kept null deliberately so that we don't get a stack blowout + // copying large record chains and code that does copy records + // can copy only a fixed number. + // this.next + + this.states = other.states; + this.filled = other.filled; + this.old_log_api = other.old_log_api; + } + + /** * Sets the well-known logging source properties. * From 11e35a998b782c9329c6683f7f35e0cf8d757195 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 1 Mar 2020 21:39:31 +1100 Subject: [PATCH 3/3] Geary.ProblemReport: Copy log records when constructed Take a copy of extant log records when constrcuted so that newer records can be dropped while the problem report still exists. This uses more memory, but limits unbounded memory growth while any log reports still exists. --- src/engine/api/geary-problem-report.vala | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/engine/api/geary-problem-report.vala b/src/engine/api/geary-problem-report.vala index 5f21b856..402d6310 100644 --- a/src/engine/api/geary-problem-report.vala +++ b/src/engine/api/geary-problem-report.vala @@ -26,8 +26,21 @@ public class Geary.ProblemReport : Object { if (error != null) { this.error = new ErrorContext(error); } - this.earliest_log = Logging.get_earliest_record(); - this.latest_log = Logging.get_latest_record(); + Logging.Record next_original = Logging.get_earliest_record(); + Logging.Record last_original = Logging.get_latest_record(); + if (next_original != null) { + Logging.Record copy = this.earliest_log = new Logging.Record.copy( + next_original + ); + next_original = next_original.next; + while (next_original != null && + next_original != last_original) { + copy.next = new Logging.Record.copy(next_original); + copy = copy.next; + next_original = next_original.next; + } + this.latest_log = copy; + } } ~ProblemReport() {