From 1c26acd815a8609314991e539dd99ceb2b9b1b43 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 30 Aug 2022 12:17:48 +0100 Subject: [PATCH] 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. --- changelog.d/13660.bugfix | 1 + synapse/rest/media/v1/_base.py | 40 ++++++++++++++++++---------------- 2 files changed, 22 insertions(+), 19 deletions(-) create mode 100644 changelog.d/13660.bugfix diff --git a/changelog.d/13660.bugfix b/changelog.d/13660.bugfix new file mode 100644 index 000000000..43859a4d6 --- /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 c35d42fab..d30878f70 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: - 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) + # 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 - # Unregister the producer, if it has one, so Twisted doesn't complain - if request.producer: - request.unregisterProducer() + 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() finish_request(request)