Optimize connection close logic to resolve timeout delay issue (#508)

apache · Nov 28, 2024 · 8ee6483 · 8ee6483
1 parent 71b98d4
commit 8ee6483
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 5 deletions.
diff --git a/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/HttpRequestExecutor.java b/httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/HttpRequestExecutor.java
@@ -54,10 +54,13 @@
 import org.apache.hc.core5.http.protocol.HttpContext;
 import org.apache.hc.core5.http.protocol.HttpCoreContext;
 import org.apache.hc.core5.http.protocol.HttpProcessor;
+import org.apache.hc.core5.io.CloseMode;
 import org.apache.hc.core5.io.Closer;
 import org.apache.hc.core5.util.Args;
 import org.apache.hc.core5.util.Timeout;
 
+import javax.net.ssl.SSLException;
+
 /**
  * {@code HttpRequestExecutor} is a client side HTTP protocol handler based
  * on the blocking (classic) I/O model.
@@ -211,9 +214,12 @@ public ClassicHttpResponse execute(
             }
             return response;
 
-        } catch (final HttpException | IOException | RuntimeException ex) {
+        } catch (final HttpException | SSLException ex) {
             Closer.closeQuietly(conn);
             throw ex;
+        } catch (final IOException | RuntimeException ex) {
+            Closer.close(conn, CloseMode.IMMEDIATE);
+            throw ex;
         }
     }
 

diff --git a/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestHttpRequestExecutor.java b/httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestHttpRequestExecutor.java
@@ -27,29 +27,31 @@
 
 package org.apache.hc.core5.http.impl.io;
 
-import java.io.IOException;
-import java.util.List;
-
 import org.apache.hc.core5.http.ClassicHttpRequest;
 import org.apache.hc.core5.http.ClassicHttpResponse;
 import org.apache.hc.core5.http.HeaderElements;
 import org.apache.hc.core5.http.HttpEntity;
 import org.apache.hc.core5.http.HttpHeaders;
 import org.apache.hc.core5.http.HttpResponse;
 import org.apache.hc.core5.http.Method;
+import org.apache.hc.core5.http.HttpException;
 import org.apache.hc.core5.http.io.HttpClientConnection;
 import org.apache.hc.core5.http.io.HttpResponseInformationCallback;
 import org.apache.hc.core5.http.message.BasicClassicHttpRequest;
 import org.apache.hc.core5.http.message.BasicClassicHttpResponse;
 import org.apache.hc.core5.http.protocol.HttpCoreContext;
 import org.apache.hc.core5.http.protocol.HttpProcessor;
+import org.apache.hc.core5.io.CloseMode;
 import org.apache.hc.core5.util.Timeout;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.mockito.ArgumentCaptor;
 import org.mockito.ArgumentMatchers;
 import org.mockito.Mockito;
 
+import java.io.IOException;
+import java.util.List;
+
 class TestHttpRequestExecutor {
 
     @Test
@@ -402,7 +404,7 @@ void testExecutionIOException() throws Exception {
 
         Mockito.doThrow(new IOException("Oopsie")).when(conn).sendRequestHeader(request);
         Assertions.assertThrows(IOException.class, () -> executor.execute(request, conn, context));
-        Mockito.verify(conn).close();
+        Mockito.verify(conn).close(CloseMode.IMMEDIATE);
     }
 
     @Test
@@ -415,6 +417,19 @@ void testExecutionRuntimeException() throws Exception {
 
         Mockito.doThrow(new RuntimeException("Oopsie")).when(conn).receiveResponseHeader();
         Assertions.assertThrows(RuntimeException.class, () -> executor.execute(request, conn, context));
+        Mockito.verify(conn).close(CloseMode.IMMEDIATE);
+    }
+
+    @Test
+    void testExecutionHttpException() throws Exception {
+        final HttpClientConnection conn = Mockito.mock(HttpClientConnection.class);
+        final HttpRequestExecutor executor = new HttpRequestExecutor();
+
+        final HttpCoreContext context = HttpCoreContext.create();
+        final ClassicHttpRequest request = new BasicClassicHttpRequest(Method.GET, "/");
+
+        Mockito.doThrow(new HttpException("Oopsie")).when(conn).receiveResponseHeader();
+        Assertions.assertThrows(HttpException.class, () -> executor.execute(request, conn, context));
         Mockito.verify(conn).close();
     }