Repository navigation
HTTP connection leaked if ExecutionInterceptor throws exception #5005
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
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
+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
Activity