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

Commit 1c26acd

Browse files
authored
Fix bug where we wedge media plugins if clients disconnect early (#13660)
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.
1 parent 303b40b commit 1c26acd

File tree

2 files changed

+22
-19
lines changed

2 files changed

+22
-19
lines changed

changelog.d/13660.bugfix

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix bug where we wedge media plugins if clients disconnect early. Introduced in v1.22.0.

synapse/rest/media/v1/_base.py

+21-19
Original file line numberDiff line numberDiff line change
@@ -254,30 +254,32 @@ async def respond_with_responder(
254254
file_size: Size in bytes of the media. If not known it should be None
255255
upload_name: The name of the requested file, if any.
256256
"""
257-
if request._disconnected:
258-
logger.warning(
259-
"Not sending response to request %s, already disconnected.", request
260-
)
261-
return
262-
263257
if not responder:
264258
respond_404(request)
265259
return
266260

267-
logger.debug("Responding to media request with responder %s", responder)
268-
add_file_headers(request, media_type, file_size, upload_name)
269-
try:
270-
with responder:
261+
# If we have a responder we *must* use it as a context manager.
262+
with responder:
263+
if request._disconnected:
264+
logger.warning(
265+
"Not sending response to request %s, already disconnected.", request
266+
)
267+
return
268+
269+
logger.debug("Responding to media request with responder %s", responder)
270+
add_file_headers(request, media_type, file_size, upload_name)
271+
try:
272+
271273
await responder.write_to_consumer(request)
272-
except Exception as e:
273-
# The majority of the time this will be due to the client having gone
274-
# away. Unfortunately, Twisted simply throws a generic exception at us
275-
# in that case.
276-
logger.warning("Failed to write to consumer: %s %s", type(e), e)
277-
278-
# Unregister the producer, if it has one, so Twisted doesn't complain
279-
if request.producer:
280-
request.unregisterProducer()
274+
except Exception as e:
275+
# The majority of the time this will be due to the client having gone
276+
# away. Unfortunately, Twisted simply throws a generic exception at us
277+
# in that case.
278+
logger.warning("Failed to write to consumer: %s %s", type(e), e)
279+
280+
# Unregister the producer, if it has one, so Twisted doesn't complain
281+
if request.producer:
282+
request.unregisterProducer()
281283

282284
finish_request(request)
283285

0 commit comments

Comments
 (0)