diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/Http2ClientImpl.java b/src/java.net.http/share/classes/jdk/internal/net/http/Http2ClientImpl.java index 6121a0c1673..c4573317785 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/Http2ClientImpl.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/Http2ClientImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2026, 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 @@ -351,4 +351,9 @@ class Http2ClientImpl { public boolean stopping() { return stopping; } + + // getConnections() is used only by tests. + Map getConnections() { + return connections; + } } diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java b/src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java index 94b8505da47..11022ca5c96 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2026, 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 @@ -52,7 +52,6 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; -import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; import java.util.function.Function; import java.util.function.Supplier; @@ -1610,16 +1609,37 @@ class Http2Connection implements Closeable { return frames; } - // Dedicated cache for headers encoding ByteBuffer. + // Dedicated reusable ByteBuffer for headers encoding. // There can be no concurrent access to this buffer as all access to this buffer // and its content happen within a single critical code block section protected - // by the sendLock. / (see sendFrame()) - // private final ByteBufferPool headerEncodingPool = new ByteBufferPool(); + // by the sendlock (see sendFrame()). + private ByteBuffer cachedHeaderBuffer; + + // getCachedHeaderBuffer() is used only by tests and it should not be + // called in source code without also holding `sendlock`. + ByteBuffer getCachedHeaderBuffer() { + return cachedHeaderBuffer; + } private ByteBuffer getHeaderBuffer(int size) { - ByteBuffer buf = ByteBuffer.allocate(size); - buf.limit(size); - return buf; + assert sendlock.isHeldByCurrentThread() : "current thread is not holding sendlock"; + + if (cachedHeaderBuffer == null || cachedHeaderBuffer.capacity() < size) { + cachedHeaderBuffer = ByteBuffer.allocate(size); + return cachedHeaderBuffer; + } + + cachedHeaderBuffer.clear(); + cachedHeaderBuffer.limit(size); + return cachedHeaderBuffer; + } + + private static ByteBuffer copyBuffer(ByteBuffer buffer) { + buffer.flip(); + ByteBuffer copy = ByteBuffer.allocate(buffer.remaining()); + copy.put(buffer); + copy.flip(); + return copy; } /* @@ -1634,8 +1654,8 @@ class Http2Connection implements Closeable { * encoding in HTTP/2... */ private List encodeHeadersImpl(int bufferSize, HttpHeaders... headers) { - ByteBuffer buffer = getHeaderBuffer(bufferSize); List buffers = new ArrayList<>(); + ByteBuffer buffer = getHeaderBuffer(bufferSize); for (HttpHeaders header : headers) { for (Map.Entry> e : header.map().entrySet()) { String lKey = e.getKey().toLowerCase(Locale.US); @@ -1644,16 +1664,17 @@ class Http2Connection implements Closeable { hpackOut.header(lKey, value); while (!hpackOut.encode(buffer)) { if (!buffer.hasRemaining()) { - buffer.flip(); - buffers.add(buffer); - buffer = getHeaderBuffer(bufferSize); + ByteBuffer copy = copyBuffer(buffer); + buffers.add(copy); + buffer.clear(); + buffer.limit(bufferSize); } } } } } - buffer.flip(); - buffers.add(buffer); + ByteBuffer copy = copyBuffer(buffer); + buffers.add(copy); return buffers; } @@ -1710,7 +1731,7 @@ class Http2Connection implements Closeable { } } - private final Lock sendlock = new ReentrantLock(); + private final ReentrantLock sendlock = new ReentrantLock(); void sendFrame(Http2Frame frame) { try { diff --git a/test/jdk/java/net/httpclient/access/java.net.http/jdk/internal/net/http/HttpClientImplAccess.java b/test/jdk/java/net/httpclient/access/java.net.http/jdk/internal/net/http/HttpClientImplAccess.java index 875ac5426dd..ce57f654e93 100644 --- a/test/jdk/java/net/httpclient/access/java.net.http/jdk/internal/net/http/HttpClientImplAccess.java +++ b/test/jdk/java/net/httpclient/access/java.net.http/jdk/internal/net/http/HttpClientImplAccess.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2025, 2026, 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 @@ -26,6 +26,9 @@ import java.lang.reflect.Field; import java.net.http.HttpClient; import java.util.Objects; import java.util.Set; +import java.nio.ByteBuffer; +import java.util.Collection; +import java.util.Map; public final class HttpClientImplAccess { @@ -64,4 +67,29 @@ public final class HttpClientImplAccess { openedConnections.setAccessible(true); return (Set) openedConnections.get(clientImpl); } + + /** + * Returns all connections in the client's HTTP/2 pool. + */ + public static Collection getHttp2Connections(final HttpClient client) { + Objects.requireNonNull(client, "client"); + final HttpClientImpl clientImpl = impl(client); + if (clientImpl == null) { + throw new IllegalStateException("Unsupported HttpClient implementation"); + } + Http2ClientImpl client2 = clientImpl.client2(); + Map connections = client2.getConnections(); + return connections.values(); + } + + /** + * Returns the cached header encoding buffer for the given Http2Connection. + */ + public static ByteBuffer getCachedHeaderBuffer(final Object conn) { + // The argument to this method is of type Object and not + // Http2Connection because callers outside the module cannot reference + // the package-private Http2Connection class. + Objects.requireNonNull(conn, "conn"); + return ((Http2Connection) conn).getCachedHeaderBuffer(); + } } diff --git a/test/jdk/java/net/httpclient/http2/HeaderEncodingBufferReuseTest.java b/test/jdk/java/net/httpclient/http2/HeaderEncodingBufferReuseTest.java new file mode 100644 index 00000000000..26fb06ec47d --- /dev/null +++ b/test/jdk/java/net/httpclient/http2/HeaderEncodingBufferReuseTest.java @@ -0,0 +1,139 @@ +/* + * Copyright (c) 2026, 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. + */ + +import java.io.IOException; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpRequest.BodyPublishers; +import java.net.http.HttpResponse; +import java.net.http.HttpResponse.BodyHandlers; +import java.nio.ByteBuffer; +import java.util.Collection; +import javax.net.ssl.SSLContext; +import static java.net.http.HttpClient.Version.HTTP_2; + +import jdk.httpclient.test.lib.common.HttpServerAdapters; +import jdk.httpclient.test.lib.common.HttpServerAdapters.HttpTestServer; +import jdk.httpclient.test.lib.common.HttpServerAdapters.HttpTestExchange; +import jdk.httpclient.test.lib.common.HttpServerAdapters.HttpTestHandler; +import jdk.internal.net.http.HttpClientImplAccess; +import jdk.test.lib.net.URIBuilder; +import jdk.test.lib.net.SimpleSSLContext; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.*; + +/* + * @test + * @bug 8383248 + * @summary Verifies that Http2Connection.cachedHeaderBuffer is reused + * across multiple requests on the same connection. + * @library /test/lib /test/jdk/java/net/httpclient/lib + * /test/jdk/java/net/httpclient/access + * @build java.net.http/jdk.internal.net.http.HttpClientImplAccess + * jdk.httpclient.test.lib.common.HttpServerAdapters + * jdk.test.lib.net.SimpleSSLContext + * @run junit/othervm + * ${test.main.class} + */ + +public class HeaderEncodingBufferReuseTest implements HttpServerAdapters { + + static String httpUri; + static SSLContext sslContext; + static HttpTestServer testServer; + + @BeforeAll + static void init() throws Exception { + sslContext = SimpleSSLContext.findSSLContext(); + testServer = HttpTestServer.create(HTTP_2, sslContext); + testServer.addHandler(new OkHandler(), "/test"); + testServer.start(); + httpUri = URIBuilder.newBuilder() + .scheme("https") + .loopback() + .port(testServer.getAddress().getPort()) + .path("/test") + .build() + .toString(); + } + + @AfterAll + static void teardown() { + testServer.stop(); + } + + @Test + void test() throws Exception { + try (HttpClient client = HttpClient.newBuilder() + .proxy(HttpClient.Builder.NO_PROXY) + .version(HttpClient.Version.HTTP_2) + .sslContext(sslContext) + .build()) { + + // Send an initial request to allocate the header encoding buffer. + assertEquals(200, send(client, httpUri, 2).statusCode()); + + Collection connections = HttpClientImplAccess.getHttp2Connections(client); + assertEquals(1, connections.size()); + Object conn = connections.iterator().next(); + + ByteBuffer cached = HttpClientImplAccess.getCachedHeaderBuffer(conn); + assertNotNull(cached); + + // Send another request and verify the same buffer is reused. + assertEquals(200, send(client, httpUri, 2).statusCode()); + connections = HttpClientImplAccess.getHttp2Connections(client); + assertEquals(1, connections.size()); + assertSame(conn, connections.iterator().next()); + assertSame(cached, HttpClientImplAccess.getCachedHeaderBuffer(conn)); + + // Verify that the buffer is reused when headers span multiple frames. + assertEquals(200, send(client, httpUri, 300).statusCode()); + connections = HttpClientImplAccess.getHttp2Connections(client); + assertEquals(1, connections.size()); + assertSame(conn, connections.iterator().next()); + assertSame(cached, HttpClientImplAccess.getCachedHeaderBuffer(conn)); + } + } + + static HttpResponse send(HttpClient client, String uri, int headerCount) throws Exception { + HttpRequest.Builder builder = HttpRequest.newBuilder(URI.create(uri)) + .POST(BodyPublishers.ofString("test")); + for (int i = 0; i < headerCount; i++) { + builder.header("X-Header-" + i, "value-" + "x".repeat(50) + "-" + i); + } + return client.send(builder.build(), BodyHandlers.discarding()); + } + + static class OkHandler implements HttpTestHandler { + @Override + public void handle(HttpTestExchange exchange) throws IOException { + exchange.sendResponseHeaders(200, 0); + exchange.getResponseBody().close(); + } + } +}