stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8288717: Add a means to close idle connections in HTTP/2 connection pool

Browse Source 
Reviewed-by: dfuchs, michaelm
This commit is contained in:
Conor Cleary 2022-11-17 08:38:25 +00:00
parent 9f8b6d2aa6
commit b9db16ab09
4 changed files with 276 additions and 5 deletions
src/java.net.http/share/classes/jdk/internal/net/http
ConnectionPool.javaHttp2Connection.javaHttpClientImpl.java
test/jdk/java/net/httpclient/http2
IdleConnectionTimeoutTest.java

9
src/java.net.http/share/classes/jdk/internal/net/http/ConnectionPool.java

@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2015, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -44,14 +44,13 @@ import java.util.stream.Collectors;
import jdk.internal.net.http.common.FlowTube;
import jdk.internal.net.http.common.Logger;
import jdk.internal.net.http.common.Utils;
import static jdk.internal.net.http.HttpClientImpl.KEEP_ALIVE_TIMEOUT; //seconds
/**
* Http 1.1 connection pool.
*/
final class ConnectionPool {
static final long KEEP_ALIVE = Utils.getIntegerNetProperty(
"jdk.httpclient.keepalive.timeout", 1200); // seconds
static final long MAX_POOL_SIZE = Utils.getIntegerNetProperty(
"jdk.httpclient.connectionPoolSize", 0); // unbounded
final Logger debug = Utils.getDebugLogger(this::dbgString, Utils.DEBUG);
@ -168,7 +167,7 @@ final class ConnectionPool {
* Returns the connection to the pool.
*/
void returnToPool(HttpConnection conn) {
returnToPool(conn, Instant.now(), KEEP_ALIVE);
returnToPool(conn, Instant.now(), KEEP_ALIVE_TIMEOUT);
}
// Called also by whitebox tests
@ -378,7 +377,7 @@ final class ConnectionPool {
// should only be called while holding a synchronization
// lock on the ConnectionPool
void add(HttpConnection conn) {
add(conn, Instant.now(), KEEP_ALIVE);
add(conn, Instant.now(), KEEP_ALIVE_TIMEOUT);
}
// Used by whitebox test.

52
src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java

@ -30,8 +30,10 @@ import java.io.IOException;
import java.io.UncheckedIOException;
import java.net.InetSocketAddress;
import java.net.URI;
import java.net.http.HttpConnectTimeoutException;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.time.Duration;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
@ -121,6 +123,7 @@ class Http2Connection {
private static final int MAX_CLIENT_STREAM_ID = Integer.MAX_VALUE; // 2147483647
private static final int MAX_SERVER_STREAM_ID = Integer.MAX_VALUE - 1; // 2147483646
private IdleConnectionTimeoutEvent idleConnectionTimeoutEvent; // may be null
/**
* Flag set when no more streams to be opened on this connection.
@ -181,6 +184,36 @@ class Http2Connection {
* and SSLTube.recycler.
*/
// An Idle connection is one that has no active streams
// and has not sent the final stream flag
final class IdleConnectionTimeoutEvent extends TimeoutEvent {
private boolean fired;
IdleConnectionTimeoutEvent(Duration duration) {
super(duration);
fired = false;
}
@Override
public void handle() {
fired = true;
if (debug.on()) {
debug.log("HTTP connection idle for too long");
}
HttpConnectTimeoutException hte = new HttpConnectTimeoutException("HTTP connection idle, no active streams. Shutting down.");
shutdown(hte);
}
@Override
public String toString() {
return "IdleConnectionTimeoutEvent, " + super.toString();
}
public boolean isFired() {
return fired;
}
}
// A small class that allows to control frames with respect to the state of
// the connection preface. Any data received before the connection
@ -982,6 +1015,8 @@ class Http2Connection {
}
}
// Check if this is the last stream aside from stream 0,
// arm timeout
void closeStream(int streamid) {
if (debug.on()) debug.log("Closed stream %d", streamid);
@ -1005,6 +1040,18 @@ class Http2Connection {
if (finalStream() && streams.isEmpty()) {
// should be only 1 stream, but there might be more if server push
close();
} else {
// Start timer if property present and not already created
synchronized (this) {
// idleConnectionTimerEvent is always accessed within a synchronized block
if (streams.isEmpty() && idleConnectionTimeoutEvent == null) {
idleConnectionTimeoutEvent = client().idleConnectionTimeout()
.map(IdleConnectionTimeoutEvent::new).orElse(null);
if (idleConnectionTimeoutEvent != null) {
client().registerTimer(idleConnectionTimeoutEvent);
}
}
}
}
}
@ -1177,6 +1224,11 @@ class Http2Connection {
}
client().streamReference();
streams.put(streamid, stream);
// idleConnectionTimerEvent is always accessed within a synchronized block
if (idleConnectionTimeoutEvent != null) {
client().cancelTimer(idleConnectionTimeoutEvent);
idleConnectionTimeoutEvent = null;
}
return;
}
}

27
src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java

@ -112,6 +112,10 @@ final class HttpClientImpl extends HttpClient implements Trackable {
final Logger debugtimeout = Utils.getDebugLogger(this::dbgString, DEBUGTIMEOUT);
static final AtomicLong CLIENT_IDS = new AtomicLong();
private final AtomicLong CONNECTION_IDS = new AtomicLong();
static final int DEFAULT_KEEP_ALIVE_TIMEOUT = 1200;
static final long KEEP_ALIVE_TIMEOUT = getTimeoutProp("jdk.httpclient.keepalive.timeout", DEFAULT_KEEP_ALIVE_TIMEOUT);
// Defaults to value used for HTTP/1 Keep-Alive Timeout. Can be overridden by jdk.httpclient.keepalive.timeout.h2 property.
static final long IDLE_CONNECTION_TIMEOUT = getTimeoutProp("jdk.httpclient.keepalive.timeout.h2", KEEP_ALIVE_TIMEOUT);
// Define the default factory as a static inner class
// that embeds all the necessary logic to avoid
@ -1544,6 +1548,10 @@ final class HttpClientImpl extends HttpClient implements Trackable {
return Optional.ofNullable(connectTimeout);
}
Optional<Duration> idleConnectionTimeout() {
return Optional.ofNullable(getIdleConnectionTimeout());
}
@Override
public Optional<ProxySelector> proxy() {
return Optional.ofNullable(userProxySelector);
@ -1708,6 +1716,25 @@ final class HttpClientImpl extends HttpClient implements Trackable {
return sslBufferSupplier;
}
private Duration getIdleConnectionTimeout() {
if (IDLE_CONNECTION_TIMEOUT >= 0)
return Duration.ofSeconds(IDLE_CONNECTION_TIMEOUT);
return null;
}
private static long getTimeoutProp(String prop, long def) {
String s = Utils.getNetProperty(prop);
try {
if (s != null) {
long timeoutVal = Long.parseLong(s);
if (timeoutVal >= 0) return timeoutVal;
}
} catch (NumberFormatException ignored) {
Log.logTrace("Invalid value set for " + prop + " property: " + ignored.toString());
}
return def;
}
// An implementation of BufferSupplier that manages a pool of
// maximum 3 direct byte buffers (SocketTube.MAX_BUFFERS) that
// are used for reading encrypted bytes off the channel before

193
test/jdk/java/net/httpclient/http2/IdleConnectionTimeoutTest.java Normal file

@ -0,0 +1,193 @@
/*
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
/*
* @test
* @bug 8288717
* @summary Tests that when the idleConnectionTimeoutEvent is configured in HTTP/2,
* an HTTP/2 connection will close within the specified interval if there
* are no active streams on the connection.
* @library /test/lib server
* @modules java.base/sun.net.www.http
* java.net.http/jdk.internal.net.http.common
* java.net.http/jdk.internal.net.http.frame
* java.net.http/jdk.internal.net.http.hpack
*
* @run testng/othervm -Djdk.httpclient.HttpClient.log=errors -Djdk.httpclient.keepalive.timeout=1
* IdleConnectionTimeoutTest
* @run testng/othervm -Djdk.httpclient.HttpClient.log=errors -Djdk.httpclient.keepalive.timeout=2
* IdleConnectionTimeoutTest
*
* @run testng/othervm -Djdk.httpclient.HttpClient.log=errors -Djdk.httpclient.keepalive.timeout.h2=1
* IdleConnectionTimeoutTest
* @run testng/othervm -Djdk.httpclient.HttpClient.log=errors -Djdk.httpclient.keepalive.timeout.h2=2
* IdleConnectionTimeoutTest
*
* @run testng/othervm -Djdk.httpclient.HttpClient.log=errors -Djdk.httpclient.keepalive.timeout.h2=1
* -Djdk.httpclient.keepalive.timeout=2
* IdleConnectionTimeoutTest
*
* @run testng/othervm -Djdk.httpclient.HttpClient.log=errors IdleConnectionTimeoutTest
* @run testng/othervm -Djdk.httpclient.HttpClient.log=errors -Djdk.httpclient.keepalive.timeout.h2=-1
* IdleConnectionTimeoutTest
* @run testng/othervm -Djdk.httpclient.HttpClient.log=errors,trace -Djdk.httpclient.keepalive.timeout.h2=abc
* IdleConnectionTimeoutTest
*/
import org.testng.annotations.BeforeTest;
import org.testng.annotations.Test;
import java.io.IOException;
import java.io.PrintStream;
import java.net.InetSocketAddress;
import java.net.URI;
import java.net.http.HttpClient;
import java.net.http.HttpRequest;
import java.net.http.HttpResponse;
import java.util.concurrent.CompletableFuture;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.net.http.HttpClient.Version.HTTP_2;
import static org.testng.Assert.assertEquals;
public class IdleConnectionTimeoutTest {
Http2TestServer http2TestServer;
URI timeoutUri;
URI noTimeoutUri;
final String IDLE_CONN_PROPERTY = "jdk.httpclient.keepalive.timeout.h2";
final String KEEP_ALIVE_PROPERTY = "jdk.httpclient.keepalive.timeout";
final String TIMEOUT_PATH = "/serverTimeoutHandler";
final String NO_TIMEOUT_PATH = "/noServerTimeoutHandler";
static final PrintStream testLog = System.err;
@BeforeTest
public void setup() throws Exception {
http2TestServer = new Http2TestServer(false, 0);
http2TestServer.addHandler(new ServerTimeoutHandler(), TIMEOUT_PATH);
http2TestServer.addHandler(new ServerNoTimeoutHandler(), NO_TIMEOUT_PATH);
http2TestServer.start();
int port = http2TestServer.getAddress().getPort();
timeoutUri = new URI("http://localhost:" + port + TIMEOUT_PATH);
noTimeoutUri = new URI("http://localhost:" + port + NO_TIMEOUT_PATH);
}
/*
If the InetSocketAddress of the first remote connection is not equal to the address of the
second remote connection, then the idleConnectionTimeoutEvent has occurred and a new connection
was made to carry out the second request by the client.
*/
@Test
public void test() throws InterruptedException {
String timeoutVal = System.getProperty(IDLE_CONN_PROPERTY);
String keepAliveVal = System.getProperty(KEEP_ALIVE_PROPERTY);
testLog.println("Test run for " + IDLE_CONN_PROPERTY + "=" + timeoutVal);
int sleepTime = 0;
HttpClient hc = HttpClient.newBuilder().version(HTTP_2).build();
HttpRequest hreq;
HttpResponse<String> hresp;
if (timeoutVal != null) {
if (keepAliveVal != null) {
// In this case, specified h2 timeout should override keep alive timeout.
// Timeout should occur
hreq = HttpRequest.newBuilder(timeoutUri).version(HTTP_2).GET().build();
sleepTime = 2000;
hresp = runRequest(hc, hreq, sleepTime);
assertEquals(hresp.statusCode(), 200, "idleConnectionTimeoutEvent was expected but did not occur");
} else if (timeoutVal.equals("1")) {
// Timeout should occur
hreq = HttpRequest.newBuilder(timeoutUri).version(HTTP_2).GET().build();
sleepTime = 2000;
hresp = runRequest(hc, hreq, sleepTime);
assertEquals(hresp.statusCode(), 200, "idleConnectionTimeoutEvent was expected but did not occur");
} else if (timeoutVal.equals("2")) {
// Timeout should not occur
hreq = HttpRequest.newBuilder(noTimeoutUri).version(HTTP_2).GET().build();
sleepTime = 1000;
hresp = runRequest(hc, hreq, sleepTime);
assertEquals(hresp.statusCode(), 200, "idleConnectionTimeoutEvent was not expected but occurred");
} else if (timeoutVal.equals("abc") || timeoutVal.equals("-1")) {
// Timeout should not occur
hreq = HttpRequest.newBuilder(noTimeoutUri).version(HTTP_2).GET().build();
hresp = runRequest(hc, hreq, sleepTime);
assertEquals(hresp.statusCode(), 200, "idleConnectionTimeoutEvent was not expected but occurred");
}
} else {
// When no value is specified then no timeout should occur (default keep alive value of 600 used)
hreq = HttpRequest.newBuilder(noTimeoutUri).version(HTTP_2).GET().build();
hresp = runRequest(hc, hreq, sleepTime);
assertEquals(hresp.statusCode(), 200, "idleConnectionTimeoutEvent should not occur, no value was specified for this property");
}
}
private HttpResponse<String> runRequest(HttpClient hc, HttpRequest req, int sleepTime) throws InterruptedException {
CompletableFuture<HttpResponse<String>> request = hc.sendAsync(req, HttpResponse.BodyHandlers.ofString(UTF_8));
HttpResponse<String> hresp = request.join();
assertEquals(hresp.statusCode(), 200);
Thread.sleep(sleepTime);
request = hc.sendAsync(req, HttpResponse.BodyHandlers.ofString(UTF_8));
return request.join();
}
static class ServerTimeoutHandler implements Http2Handler {
InetSocketAddress remote;
@Override
public void handle(Http2TestExchange exchange) throws IOException {
if (remote == null) {
remote = exchange.getRemoteAddress();
exchange.sendResponseHeaders(200, 0);
} else if (!remote.equals(exchange.getRemoteAddress())) {
testLog.println("ServerTimeoutHandler: New Connection was used, idleConnectionTimeoutEvent fired."
+ " First remote: " + remote + ", Second Remote: " + exchange.getRemoteAddress());
exchange.sendResponseHeaders(200, 0);
} else {
exchange.sendResponseHeaders(400, 0);
}
}
}
static class ServerNoTimeoutHandler implements Http2Handler {
InetSocketAddress oldRemote;
@Override
public void handle(Http2TestExchange exchange) throws IOException {
InetSocketAddress newRemote = exchange.getRemoteAddress();
if (oldRemote == null) {
oldRemote = newRemote;
exchange.sendResponseHeaders(200, 0);
} else if (oldRemote.equals(newRemote)) {
testLog.println("ServerNoTimeoutHandler: Same Connection was used, idleConnectionTimeoutEvent did not fire."
+ " First remote: " + oldRemote + ", Second Remote: " + newRemote);
exchange.sendResponseHeaders(200, 0);
} else {
exchange.sendResponseHeaders(400, 0);
}
}
}
}