HTTP connection leaked if ExecutionInterceptor throws exception
- Dominant language
- Java
- Stars
- 2.6k
- Forks
- 1k
- Avg merge
- 2d 9h
- Merged PRs (30d)
- 51
Description
### Describe the bug
When handling a response with a body, if an `ExecutionInterceptor.modifyHttpResponse` or `ExecutionInterceptor.modifyHttpResponseContent` method throws an exception, the HTTP connection is never closed.
### Expected Behavior
Any time an AwsClient isn't returning the response stream to the caller, it should close the response itself, releasing the connection back to the pool.
### Current Behavior
The connection remains open. This eventually exhausts the connection pool and causes subsequent requests to hang forever.
### Reproduction Steps
```java
import com.sun.net.httpserver.HttpServer;
import java.io.IOException;
import java.net.InetSocketAddress;
import java.util.List;
import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration;
import software.amazon.awssdk.core.interceptor.Context;
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
import software.amazon.awssdk.core.interceptor.ExecutionInterceptor;
import software.amazon.awssdk.http.SdkHttpRequest;
import software.amazon.awssdk.http.SdkHttpResponse;
import software.amazon.awssdk.services.s3.S3Client;
public class ConnectionLeakDemo {
public static void main(String[] args) throws IOException {
HttpServer dummyServer = HttpServer.create(new InetSocketAddress(0), 0);
ExecutionInterceptor interceptor = new ExecutionInterceptor() {
@Override
public SdkHttpRequest modifyHttpRequest(Context.ModifyHttpRequest context, ExecutionAttributes executionAttributes) {
return context.httpRequest().toBuilder()
.protocol("http").host("localhost").port(dummyServer.getAddress().getPort())
.build();
}
@Override
public SdkHttpResponse modifyHttpResponse(Context.ModifyHttpResponse context, ExecutionAttributes executionAttributes) {
throw new RuntimeException("Throwing exception from modifyHttpResponse");
}
};
dummyServer.start();
try (S3Client client = S3Client.builder()
.overrideConfiguration(ClientOverrideConfiguration.builder().executionInterceptors(List.of(interceptor)).build())
.build()) {
for (int i = 1; i <= 100; i++) {
System.out.print("Request #" + i + ": ");
try (var in = client.getObject(b -> b.bucket("bucket").key("key"))) {
System.out.println(in);
} catch (Exception e) {
System.out.println(e);
}
}
}
dummyServer.stop(0);
}
}
```
After the 50th exception, `getObject` gets stuck waiting for a connection.
### Possible Solution
`ExecutionInterceptorChain.modifyHttpResponse` could catch all exceptions, and close the current response stream before rethrowing.
Something like
```diff
+InputStream response = context.responseBody().orElse(null);
+try {
InterceptorContext result = context;
for (int i = interceptors.size() - 1; i >= 0; i--) {
SdkHttpResponse interceptorResult = interceptors.get(i).modifyHttpResponse(result, executionAttributes);
validateInterceptorResult(result.httpResponse(), interceptorResult, interceptors.get(i), "modifyHttpResponse");
- InputStream response = interceptors.get(i).modifyHttpResponseContent(result, executionAttributes).orElse(null);
+ response = interceptors.get(i).modifyHttpResponseContent(result, executionAttributes).orElse(null);
result = result.toBuilder().httpResponse(interceptorResult).responseBody(response).build();
}
return result;
+} catch (Throwable t) {
+ IoUtils.closeQuietly(response);
+ throw t;
+}
```
The problem probably exists in other places, though. I'm not familiar enough with this system to say what a complete solution should be.
### Additional Information/Context
It's possible to work around this issue by wrapping all of your own `modifyHttpResponse`/`modifyHttpResponseContent` implementations in try/catch/close/rethrow. However, I don't think it's reasonable to expect these methods to close a stream that they didn't create. `modifyHttpResponse` doesn't even use the stream normally.
### AWS Java SDK version used
2.23.4
### JDK version used
openjdk version "17.0.10" 2024-01-16
### Operating System and version
Ubuntu 20.04
Contributor guide
Assessment
This issue has not been assessed yet.