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

Commit

Permalink
Fix bug where we wedge media plugins if clients disconnect early (#13660
Browse files Browse the repository at this point in the history
)

We incorrectly didn't use the returned `Responder` if the client had
disconnected, which meant that the resource used by the Responder
wasn't correctly released.

In particular, this exhausted the thread pools so that *all* requests
timed out.
  • Loading branch information
erikjohnston authored Aug 30, 2022
1 parent 303b40b commit 1c26acd
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 19 deletions.
1 change: 1 addition & 0 deletions changelog.d/13660.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix bug where we wedge media plugins if clients disconnect early. Introduced in v1.22.0.
40 changes: 21 additions & 19 deletions synapse/rest/media/v1/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,30 +254,32 @@ async def respond_with_responder(
file_size: Size in bytes of the media. If not known it should be None
upload_name: The name of the requested file, if any.
"""
if request._disconnected:
logger.warning(
"Not sending response to request %s, already disconnected.", request
)
return

if not responder:
respond_404(request)
return

logger.debug("Responding to media request with responder %s", responder)
add_file_headers(request, media_type, file_size, upload_name)
try:
with responder:
# If we have a responder we *must* use it as a context manager.
with responder:
if request._disconnected:
logger.warning(
"Not sending response to request %s, already disconnected.", request
)
return

logger.debug("Responding to media request with responder %s", responder)
add_file_headers(request, media_type, file_size, upload_name)
try:

await responder.write_to_consumer(request)
except Exception as e:
# The majority of the time this will be due to the client having gone
# away. Unfortunately, Twisted simply throws a generic exception at us
# in that case.
logger.warning("Failed to write to consumer: %s %s", type(e), e)

# Unregister the producer, if it has one, so Twisted doesn't complain
if request.producer:
request.unregisterProducer()
except Exception as e:
# The majority of the time this will be due to the client having gone
# away. Unfortunately, Twisted simply throws a generic exception at us
# in that case.
logger.warning("Failed to write to consumer: %s %s", type(e), e)

# Unregister the producer, if it has one, so Twisted doesn't complain
if request.producer:
request.unregisterProducer()

finish_request(request)

Expand Down

0 comments on commit 1c26acd

Please sign in to comment.