aws / aws/aws-sdk-java-v2

HTTP connection leaked if ExecutionInterceptor throws exception

Open
#5,005 0 comments 2 reactions 0 assignees View on GitHub
bug p2
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.