Fix an issue that could result in resource leak when sending request …

…fails due to errors such as invalid request. (#3719)
aws · Jan 26, 2023 · ee51283 · ee51283
1 parent d66db22
commit ee51283
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 10 deletions.
diff --git a/.changes/next-release/bugfix-AWSCRTHTTPClient-3f05324.json b/.changes/next-release/bugfix-AWSCRTHTTPClient-3f05324.json
@@ -0,0 +1,6 @@
+{
+    "type": "bugfix",
+    "category": "AWS CRT HTTP Client",
+    "contributor": "",
+    "description": "Fix an issue that could result in resource leak when sending request fails due to errors such as invalid request."
+}
diff --git a/...crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java b/...crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java
@@ -73,7 +73,8 @@ public CompletableFuture<Void> execute(CrtRequestContext executionContext) {
 
             // If we didn't get a connection for some reason, fail the request
             if (throwable != null) {
-                reportFailure(new IOException("An exception occurred when acquiring a connection", throwable),
+                reportFailure(crtConn,
+                              new IOException("An exception occurred when acquiring a connection", throwable),
                               requestFuture,
                               asyncRequest.responseHandler());
                 return;
@@ -121,12 +122,13 @@ private void executeRequest(CrtRequestContext executionContext,
                 // it's semantically an IOException anyway.
                 toThrow = new IOException(e);
             }
-            reportFailure(toThrow,
+            reportFailure(crtConn,
+                          toThrow,
                           requestFuture,
                           asyncRequest.responseHandler());
         } catch (IllegalStateException | CrtRuntimeException e) {
             // CRT throws IllegalStateException if the connection is closed
-            reportFailure(new IOException("An exception occurred when making the request", e),
+            reportFailure(crtConn, new IOException("An exception occurred when making the request", e),
                           requestFuture,
                           asyncRequest.responseHandler());
         }
@@ -156,9 +158,14 @@ private CompletableFuture<Void> createExecutionFuture(AsyncExecuteRequest reques
     /**
      * Notify the provided response handler and future of the failure.
      */
-    private void reportFailure(Throwable cause,
+    private void reportFailure(HttpClientConnection crtConn,
+                               Throwable cause,
                                CompletableFuture<Void> executeFuture,
                                SdkAsyncHttpResponseHandler responseHandler) {
+        if (crtConn != null) {
+            crtConn.close();
+        }
+
         try {
             responseHandler.onError(cause);
         } catch (Exception e) {

diff --git a/...ts/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/H1ServerBehaviorTest.java b/...ts/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/H1ServerBehaviorTest.java
@@ -19,7 +19,6 @@
 
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
-import org.junit.jupiter.api.Test;
 import software.amazon.awssdk.crt.CrtResource;
 import software.amazon.awssdk.crt.Log;
 import software.amazon.awssdk.http.SdkAsyncHttpClientH1TestSuite;
@@ -47,9 +46,4 @@ protected SdkAsyncHttpClient setupClient() {
         return AwsCrtAsyncHttpClient.builder()
                                     .buildWithDefaults(AttributeMap.builder().put(TRUST_ALL_CERTIFICATES, true).build());
     }
-
-    @Test
-    // TODO: Remove this override. Temporarily disabling test because it's causing memory leak
-    public void naughtyHeaderCharactersDoNotGetToServer() {
-    }
 }