summary refs log tree commit diff
diff options
context:
space:
mode:
authorErik Johnston <erik@matrix.org>2022-08-30 12:17:48 +0100
committerGitHub <noreply@github.com>2022-08-30 12:17:48 +0100
commit1c26acd815a8609314991e539dd99ceb2b9b1b43 (patch)
treeb884c66bd004a73c47181230a568020a26d2236d
parentDo not wait for background updates to complete do expire URL cache. (#13657) (diff)
downloadsynapse-1c26acd815a8609314991e539dd99ceb2b9b1b43.tar.xz
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.
-rw-r--r--changelog.d/13660.bugfix1
-rw-r--r--synapse/rest/media/v1/_base.py40
2 files changed, 22 insertions, 19 deletions
diff --git a/changelog.d/13660.bugfix b/changelog.d/13660.bugfix
new file mode 100644

index 0000000000..43859a4d65 --- /dev/null +++ b/changelog.d/13660.bugfix
@@ -0,0 +1 @@ +Fix bug where we wedge media plugins if clients disconnect early. Introduced in v1.22.0. diff --git a/synapse/rest/media/v1/_base.py b/synapse/rest/media/v1/_base.py
index c35d42fab8..d30878f704 100644 --- a/synapse/rest/media/v1/_base.py +++ b/synapse/rest/media/v1/_base.py
@@ -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)