Fix CRT HTTP client connection leak when aborting response stream#6876
Fix CRT HTTP client connection leak when aborting response stream#6876
Conversation
The stream manager refactoring replaced connection.shutdown() +
connection.close() with stream.close(), which only releases the
refcount without forcing the connection to shut down. This caused
connections to leak when customers called abort() before fully
consuming a GetObject response stream.
Split ResponseHandlerHelper.closeStream() into two methods:
- releaseConnection(): calls stream.close() to return the connection
to the pool (used on successful completion)
- closeConnection(): calls stream.cancel() then stream.close() to
force-shutdown the connection (used on error/abort paths)
Bump aws-crt version to 0.45.1 for the new stream.cancel() API.
| responsePublisher.complete().whenComplete((result, failure) -> { | ||
| if (failure != null) { | ||
| failResponseHandlerAndFuture(failure); | ||
| responseHandlerHelper.closeStream(); |
There was a problem hiding this comment.
I removed this because it's a no-op because releaseConnection is invoked at line 125 anyway
|
|
||
| crtResponseHandler.onResponseComplete(httpStream, 0); | ||
| assertThatThrownBy(() -> requestFuture.join()).isInstanceOf(CancellationException.class).hasMessageContaining( | ||
| assertThatThrownBy(() -> requestFuture.join()).isInstanceOf(CancellationException.class).hasStackTraceContaining( |
There was a problem hiding this comment.
Changed this to make the build pass in Java 25. In Java 25, the error message is just "join".
java.util.concurrent.CancellationException: join
at java.base/java.util.concurrent.CompletableFuture.reportJoin(CompletableFuture.java:454)
at java.base/java.util.concurrent.CompletableFuture.join(CompletableFuture.java:2139)
at software.amazon.awssdk.http.crt.internal.CrtResponseHandlerTest.onResponseComplete_publisherCancelled_closesStream(CrtResponseHandlerTest.java:71)
at java.base/java.lang.reflect.Method.invoke(Method.java:565)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)
Caused by: java.util.concurrent.CancellationException: subscription has been cancelled.
| stubFor(get(anyUrl()) | ||
| .inScenario("SucceedThenFail") | ||
| .whenScenarioStateIs("first request") | ||
| .willSetStateTo("second request") |
There was a problem hiding this comment.
This test has always been broken; the WireMock scenario only had stubs for two GET requests, but the CRT S3 client retries the faulted request multiple times, causing subsequent retries to hit an unmatched stub and receive a 404.
Previously, the tiny buffer pool (from initialReadBufferSizeInBytes(5L)) couldn't hold the 404 error body, masking it as a socket error; CRT 0.45.1 no longer reuses pooled buffers for error responses, so the 404 is now correctly parsed as an S3Exception.
Fixed the test so that we are testing the intended behavior
|
|
This pull request has been closed and the conversation has been locked. Comments on closed PRs are hard for our team to see. If you need more assistance, please open a new issue that references this one. |
4 participants
Motivation and Context
Fix connection leak in CRT HTTP client when aborting response streams.
The stream manager refactoring replaced
connection.shutdown()+connection.close()withstream.close(), which only releases the refcount without forcing the connection to shut down.This caused connections to leak when customers called
abort()before fully consuming aGetObjectresponse stream, eventually leading toHttpException: Connection Manager failed to acquire a connection within the defined timeout.Modifications
ResponseHandlerHelper.closeStream()into two methods:releaseConnection()— callsstream.close()to return the connection to the pool. Used when the response completes successfully and the stream has beenfully consumed.
closeConnection()— callsstream.cancel()thenstream.close()to force-shutdown the connection. Used when:onResponseCompleteCrtResponseAdapterandInputStreamAdaptingHttpStreamResponseHandlerto call the appropriate method on each pathaws-crtversion to 0.45.1 for the newstream.cancel()APITesting
Added
ResponseHandlerHelperTest— unit tests for ordering, idempotency, and mutual exclusion ofreleaseConnection/closeConnectionUpdated existing handler tests (
BaseHttpStreamResponseHandlerTest,InputStreamAdaptingHttpStreamResponseHandlerTest) to verifycancel()+close()ordering on error/abort paths
Added
GetObjectResponseInputStreamConnectionManagementTest— functional tests with WireMock parameterized across all HTTP clients (Apache, UrlConnection, CRTsync, Netty, CRT async) covering both abort and happy-path (fully consumed) scenarios
Added unit tests
Added functional tests
Ran existing tests
Verified fix against reproduction case from ticket
Types of changes
Checklist
mvn clean install -pl :{module}succeeds for affected modulesLaunchChangelog
License