From e4d1cff6597ac25d435fe16e0fc49d23f6e65df4 Mon Sep 17 00:00:00 2001 From: Darragh Clarke Date: Thu, 9 Feb 2023 12:27:57 +0000 Subject: [PATCH] 8300268: ServerImpl allows too many idle connections when using sun.net.httpserver.maxIdleConnections Reviewed-by: dfuchs, vtewari, michaelm, jpai --- .../sun/net/httpserver/ServerImpl.java | 25 ++- .../bugs/8300268/MaxIdleConnectionsTest.java | 145 ++++++++++++++++++ .../sun/net/httpserver/HttpServerAccess.java | 50 ++++++ 3 files changed, 215 insertions(+), 5 deletions(-) create mode 100644 test/jdk/com/sun/net/httpserver/bugs/8300268/MaxIdleConnectionsTest.java create mode 100644 test/jdk/com/sun/net/httpserver/bugs/8300268/jdk.httpserver/sun/net/httpserver/HttpServerAccess.java diff --git a/src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java b/src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java index 76153176595..530ead86c94 100644 --- a/src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java +++ b/src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2023, 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 @@ -410,7 +410,7 @@ class ServerImpl { } } responseCompleted (c); - if (t.close || idleConnections.size() >= MAX_IDLE_CONNECTIONS) { + if (t.close) { c.close(); allConnections.remove (c); } else { @@ -961,9 +961,24 @@ class ServerImpl { } void markIdle(HttpConnection c) { - c.idleStartTime = System.currentTimeMillis(); - c.setState(State.IDLE); - idleConnections.add(c); + boolean close = false; + + synchronized(idleConnections) { + if (idleConnections.size() >= MAX_IDLE_CONNECTIONS) { + // closing the connection here could block + // instead set boolean and close outside the synchronized block + close = true; + } else { + c.idleStartTime = System.currentTimeMillis(); + c.setState(State.IDLE); + idleConnections.add(c); + } + } + + if (close) { + c.close(); + allConnections.remove(c); + } } void markNewlyAccepted(HttpConnection c) { diff --git a/test/jdk/com/sun/net/httpserver/bugs/8300268/MaxIdleConnectionsTest.java b/test/jdk/com/sun/net/httpserver/bugs/8300268/MaxIdleConnectionsTest.java new file mode 100644 index 00000000000..015f0773b07 --- /dev/null +++ b/test/jdk/com/sun/net/httpserver/bugs/8300268/MaxIdleConnectionsTest.java @@ -0,0 +1,145 @@ +/* + * Copyright (c) 2023, 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. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * 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 8300268 + * @library /test/lib + * @modules jdk.httpserver/sun.net.httpserver + * @build jdk.httpserver/sun.net.httpserver.HttpServerAccess MaxIdleConnectionsTest + * @run junit/othervm -Dsun.net.httpserver.maxIdleConnections=4 MaxIdleConnectionsTest + */ + +import com.sun.net.httpserver.HttpServer; +import jdk.test.lib.net.URIBuilder; +import sun.net.httpserver.HttpServerAccess; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.io.InputStream; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.URL; +import java.net.URLConnection; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.*; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.junit.jupiter.api.Assertions.assertTrue; + +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public class MaxIdleConnectionsTest { + + HttpServer server; + int maxIdleConnections, totalConnections; + CountDownLatch reqFinishedProcessing; + + @BeforeAll + void before() throws Exception { + maxIdleConnections = Integer.getInteger("sun.net.httpserver.maxIdleConnections"); + totalConnections = maxIdleConnections + 1; + reqFinishedProcessing = new CountDownLatch(totalConnections); + server = startServer(reqFinishedProcessing); + } + + @AfterAll + void after() throws Exception { + server.stop(0); + } + + // Issue one too many requests and assert that the idle connection pool doesn't + // exceed maxIdleConnections + @Test + public void test() throws Exception { + final int port = server.getAddress().getPort(); + + final List> responses = new ArrayList<>(); + try (final ExecutorService requestIssuer = Executors.newFixedThreadPool(totalConnections)) { + for (int i = 1; i <= totalConnections; i++) { + URL requestURL = URIBuilder.newBuilder() + .scheme("http") + .loopback() + .port(port) + .path("/MaxIdleConnectionTest/" + i) + .toURL(); + final Future result = requestIssuer.submit(() -> { + System.out.println("Issuing request " + requestURL); + final URLConnection conn = requestURL.openConnection(); + try (final InputStream is = conn.getInputStream()) { + is.readAllBytes(); + } + return null; + }); + responses.add(result); + } + // wait for all the requests to reach each of the handlers + System.out.println("Waiting for all " + totalConnections + " requests to reach" + + " the server side request handler"); + reqFinishedProcessing.await(); + } + + // verify every request got served before checking idle count + for (int i = 0; i < totalConnections; i++) { + responses.get(i).get(); + System.out.println("Received successful response for request " + i); + } + + // assert that the limit set by maxIdleConnections was not exceeded + int idleConnectionCount = HttpServerAccess.getIdleConnectionCount(server); + System.out.println("count " + idleConnectionCount); + assertTrue(maxIdleConnections >= idleConnectionCount, + String.format("Too many idle connections: %d, limit: %d", idleConnectionCount, maxIdleConnections)); + } + + // Create HttpServer that will handle requests with multiple threads + private static HttpServer startServer(final CountDownLatch reqFinishedProcessing) throws IOException { + final var bindAddr = new InetSocketAddress(InetAddress.getLoopbackAddress(), 0); + final HttpServer server = HttpServer.create(bindAddr, 0); + + final AtomicInteger threadId = new AtomicInteger(); + server.setExecutor(Executors.newCachedThreadPool(r -> { + final Thread t = new Thread(r); + t.setName("http-request-handler-" + threadId.incrementAndGet()); + t.setDaemon(true); + return t; + })); + + server.createContext("/MaxIdleConnectionTest/", (exchange) -> { + System.out.println("Request " + exchange.getRequestURI() + " received"); + System.out.println("Sending response for request " + exchange.getRequestURI() + " from " + exchange.getRemoteAddress()); + reqFinishedProcessing.countDown(); + exchange.sendResponseHeaders(200, 0); + exchange.getResponseBody().close(); + }); + + server.start(); + System.out.println("Server started at address " + server.getAddress()); + return server; + } +} diff --git a/test/jdk/com/sun/net/httpserver/bugs/8300268/jdk.httpserver/sun/net/httpserver/HttpServerAccess.java b/test/jdk/com/sun/net/httpserver/bugs/8300268/jdk.httpserver/sun/net/httpserver/HttpServerAccess.java new file mode 100644 index 00000000000..7df9426381c --- /dev/null +++ b/test/jdk/com/sun/net/httpserver/bugs/8300268/jdk.httpserver/sun/net/httpserver/HttpServerAccess.java @@ -0,0 +1,50 @@ +/* + * Copyright (c) 2023, 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. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * 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. + */ + +package sun.net.httpserver; + +import com.sun.net.httpserver.HttpServer; +import java.lang.reflect.Field; +import java.util.Set; + +public class HttpServerAccess { + + // Given a HttpServer object get the number of idleConnections it currently has + public static int getIdleConnectionCount(HttpServer server) throws Exception{ + // Use reflection to get server object which is HTTPServerImpl + Field serverField = server.getClass().getDeclaredField("server"); + serverField.setAccessible(true); + + // Get the actual serverImpl class, then get the IdleConnection Field + Object serverImpl = serverField.get(server); + Field idleConnectionField = serverImpl.getClass().getDeclaredField("idleConnections"); + idleConnectionField.setAccessible(true); + + // Finally get the IdleConnection object which is of type Set + Object idleConnectionSet = idleConnectionField.get(serverImpl); + Set idleConnectionPool = (Set) idleConnectionSet; + return idleConnectionPool.size(); + } +}