diff options
| author | Erik Johnston <erik@matrix.org> | 2022-08-30 12:17:48 +0100 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2022-08-30 12:17:48 +0100 |
| commit | 1c26acd815a8609314991e539dd99ceb2b9b1b43 (patch) | |
| tree | b884c66bd004a73c47181230a568020a26d2236d | |
| parent | Do not wait for background updates to complete do expire URL cache. (#13657) (diff) | |
| download | synapse-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.bugfix | 1 | ||||
| -rw-r--r-- | synapse/rest/media/v1/_base.py | 40 |
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) |