Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Merge pull request #3723 from matrix-org/rav/fix_logcontext_disaster
Browse files Browse the repository at this point in the history
Fix exceptions when a connection is closed before we read the headers
  • Loading branch information
hawkowl committed Aug 20, 2018
2 parents 80bf7d3 + 012d612 commit 23d7e63
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 3 deletions.
1 change: 1 addition & 0 deletions changelog.d/3723.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix bug in v0.33.3rc1 which caused infinite loops and OOMs
8 changes: 7 additions & 1 deletion synapse/http/site.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
12 changes: 10 additions & 2 deletions synapse/util/logcontext.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -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):
Expand Down

0 comments on commit 23d7e63

Please sign in to comment.