Skip to content

Optimize connection close logic to resolve timeout delay issue #508

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ok2c merged 7 commits into from
Nov 28, 2024
Merged

Optimize connection close logic to resolve timeout delay issue #508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f87a81b
Add option to close connections with RST on timeout
ooeunz Nov 27, 2024
c66648c
Add version tag
ooeunz Nov 27, 2024
0c2c722
Change default behavior to IMMEDIATE close
ooeunz Nov 27, 2024
1b2709a
Separate connection close handling for IOException and regular except…
ooeunz Nov 27, 2024
4bd75b9
Use closeQuietly for SSLException handling
ooeunz Nov 27, 2024
7247895
Fix code style
ooeunz Nov 27, 2024
d243833
Replace wildcard imports with explicit imports
ooeunz Nov 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion httpcore5/src/main/java/org/apache/hc/core5/http/impl/io/HttpRequestExecutor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}
}

Expand Down
23 changes: 19 additions & 4 deletions httpcore5/src/test/java/org/apache/hc/core5/http/impl/io/TestHttpRequestExecutor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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();
}

Expand Down