diff --git a/changelog.d/3723.bugfix b/changelog.d/3723.bugfix new file mode 100644 index 000000000000..7c21083691f3 --- /dev/null +++ b/changelog.d/3723.bugfix @@ -0,0 +1 @@ +Fix bug in v0.33.3rc1 which caused infinite loops and OOMs diff --git a/synapse/http/site.py b/synapse/http/site.py index ad2a98468e62..88ed3714f9be 100644 --- a/synapse/http/site.py +++ b/synapse/http/site.py @@ -182,7 +182,7 @@ def connectionLost(self, reason): # the client disconnects. with PreserveLoggingContext(self.logcontext): logger.warn( - "Error processing request: %s %s", reason.type, reason.value, + "Error processing request %r: %s %s", self, reason.type, reason.value, ) if not self._is_processing: @@ -219,6 +219,12 @@ def _finished_processing(self): """Log the completion of this request and update the metrics """ + if self.logcontext is None: + # this can happen if the connection closed before we read the + # headers (so render was never called). In that case we'll already + # have logged a warning, so just bail out. + return + usage = self.logcontext.get_resource_usage() if self._processing_finished_time is None: diff --git a/synapse/util/logcontext.py b/synapse/util/logcontext.py index 07e83fadda0c..a0c2d37610ea 100644 --- a/synapse/util/logcontext.py +++ b/synapse/util/logcontext.py @@ -385,7 +385,13 @@ def filter(self, record): context = LoggingContext.current_context() for key, value in self.defaults.items(): setattr(record, key, value) - context.copy_to(record) + + # context should never be None, but if it somehow ends up being, then + # we end up in a death spiral of infinite loops, so let's check, for + # robustness' sake. + if context is not None: + context.copy_to(record) + return True @@ -396,7 +402,9 @@ class PreserveLoggingContext(object): __slots__ = ["current_context", "new_context", "has_parent"] - def __init__(self, new_context=LoggingContext.sentinel): + def __init__(self, new_context=None): + if new_context is None: + new_context = LoggingContext.sentinel self.new_context = new_context def __enter__(self):