From 8ee64839a8aefb805f79379d54fa27501179d764 Mon Sep 17 00:00:00 2001 From: Noah Date: Thu, 28 Nov 2024 16:38:45 +0900 Subject: [PATCH] Optimize connection close logic to resolve timeout delay issue (#508) --- .../http/impl/io/HttpRequestExecutor.java | 8 ++++++- .../http/impl/io/TestHttpRequestExecutor.java | 23 +++++++++++++++---- 2 files changed, 26 insertions(+), 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 index a75ca65ba..9bff2058e 100644 --- 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 index d961c5a28..f9451bf82 100644 --- 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,9 +27,6 @@ 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; @@ -37,12 +34,14 @@ 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; @@ -50,6 +49,9 @@ 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(); }