[#794] Wait for the listen port to be open before returning from connection handler start (#796)
6 files modified
1 files added
| | |
| | | */ |
| | | private final Object waitListen = new Object(); |
| | | |
| | | /** |
| | | * Condition predicate for {@link #waitListen}: set once the handler thread has attempted to open the listen socket |
| | | * (successfully or not). Guarded by the {@link #waitListen} monitor; protects the start method against spurious |
| | | * wakeups. |
| | | */ |
| | | private boolean listenAttempted; |
| | | |
| | | /** The friendly name of this connection handler. */ |
| | | private String friendlyName; |
| | | |
| | |
| | | } |
| | | |
| | | /** |
| | | * {@inheritDoc} |
| | | * <p> |
| | | * Deliberately left unsynchronized, unlike the overridden {@link Thread#start()}: the state this method touches is |
| | | * guarded by the {@link #waitListen} monitor, and holding the thread's own monitor across {@code waitListen.wait()} |
| | | * would block {@link Thread#join()}. |
| | | */ |
| | | @Override |
| | | public void start() { |
| | | // The Directory Server start process should only return when the connection handler port is fully opened |
| | | // and working. The start method therefore needs to wait for the created thread. |
| | | synchronized (waitListen) { |
| | | super.start(); |
| | | |
| | | final long deadlineNanos = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(LISTEN_TIMEOUT_MS); |
| | | try { |
| | | while (!listenAttempted) { |
| | | final long remainingMs = TimeUnit.NANOSECONDS.toMillis(deadlineNanos - System.nanoTime()); |
| | | if (remainingMs <= 0) { |
| | | logger.error(ERR_CONNHANDLER_TIMEOUT_WAITING_FOR_LISTENER, handlerName, |
| | | TimeUnit.MILLISECONDS.toSeconds(LISTEN_TIMEOUT_MS)); |
| | | break; |
| | | } |
| | | waitListen.wait(remainingMs); |
| | | } |
| | | } catch (InterruptedException e) { |
| | | // If something interrupted the start its probably better to return ASAP. |
| | | Thread.currentThread().interrupt(); |
| | | } |
| | | } |
| | | } |
| | | |
| | | /** |
| | | * Operates in a loop, accepting new connections and ensuring that requests on those connections are handled |
| | | * properly. |
| | | */ |
| | |
| | | // so notify here to allow the server startup to complete. |
| | | synchronized (waitListen) { |
| | | starting = false; |
| | | listenAttempted = true; |
| | | waitListen.notifyAll(); |
| | | } |
| | | } |
| | |
| | | } |
| | | |
| | | try { |
| | | // At this point, the connection Handler either started correctly or failed |
| | | // to start but the start process should be notified and resume its work in any cases. |
| | | synchronized (waitListen) { |
| | | waitListen.notifyAll(); |
| | | } |
| | | |
| | | // If we have gotten here, then we are about to start listening |
| | | // for the first time since startup or since we were previously disabled. |
| | | startListener(); |
| | |
| | | } else { |
| | | lastIterationFailed = true; |
| | | } |
| | | } finally { |
| | | // At this point, the connection handler either started listening or failed to do so, |
| | | // but the start process should be notified and resume its work in any cases. |
| | | synchronized (waitListen) { |
| | | listenAttempted = true; |
| | | waitListen.notifyAll(); |
| | | } |
| | | } |
| | | } |
| | | |
| | |
| | | * |
| | | * Copyright 2006-2009 Sun Microsystems, Inc. |
| | | * Portions Copyright 2012-2016 ForgeRock AS. |
| | | * Portions Copyright 2026 3A Systems, LLC. |
| | | */ |
| | | package org.opends.server.api; |
| | | |
| | |
| | | import java.util.Collection; |
| | | import java.util.Collections; |
| | | import java.util.List; |
| | | import java.util.concurrent.TimeUnit; |
| | | |
| | | import org.forgerock.i18n.LocalizableMessage; |
| | | import org.forgerock.opendj.server.config.server.ConnectionHandlerCfg; |
| | |
| | | |
| | | private static final LocalizedLogger logger = LocalizedLogger.getLoggerForThisClass(); |
| | | |
| | | /** |
| | | * Maximum time a {@code start()} implementation waits for its handler thread |
| | | * to attempt to open the listen socket. The wait is bounded so that a handler |
| | | * thread dying before it reaches the listen code cannot hang the whole server |
| | | * startup. |
| | | * <p> |
| | | * {@code DirectoryServer.startConnectionHandlers()} starts the handlers one |
| | | * after another, so the worst case for a start is this value multiplied by |
| | | * the number of configured handlers. It is therefore kept far below the |
| | | * {@code start-ds} timeout ({@code DirectoryServer.DEFAULT_TIMEOUT}, 200 |
| | | * seconds), which is generous for a {@code bind()}. |
| | | */ |
| | | protected static final long LISTEN_TIMEOUT_MS = TimeUnit.SECONDS.toMillis(15); |
| | | |
| | | /** The monitor associated with this connection handler. */ |
| | | private ConnectionHandlerMonitor monitor; |
| | | |
| | |
| | | private final Object waitListen = new Object(); |
| | | |
| | | /** |
| | | * The condition guarding {@link #waitListen}: set once the run method has tried to open the |
| | | * socket port, whether it succeeded or not. Guarded by {@link #waitListen}. |
| | | * Condition predicate for {@link #waitListen}: set once the handler thread |
| | | * has attempted to start the embedded HTTP server (successfully or not). |
| | | * Guarded by the {@link #waitListen} monitor; protects the start method |
| | | * against spurious wakeups. |
| | | */ |
| | | private boolean listenAttempted; |
| | | |
| | |
| | | return httpServer != null; |
| | | } |
| | | |
| | | /** |
| | | * {@inheritDoc} |
| | | * <p> |
| | | * Deliberately left unsynchronized, unlike the overridden {@link Thread#start()}: |
| | | * the state this method touches is guarded by the {@link #waitListen} monitor, and |
| | | * holding the thread's own monitor across {@code waitListen.wait()} would block |
| | | * {@link Thread#join()}. |
| | | */ |
| | | @Override |
| | | public void start() |
| | | { |
| | |
| | | { |
| | | super.start(); |
| | | |
| | | final long deadlineNanos = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(LISTEN_TIMEOUT_MS); |
| | | try |
| | | { |
| | | while (!listenAttempted) |
| | | { |
| | | waitListen.wait(); |
| | | final long remainingMs = TimeUnit.NANOSECONDS.toMillis(deadlineNanos - System.nanoTime()); |
| | | if (remainingMs <= 0) |
| | | { |
| | | logger.error(ERR_CONNHANDLER_TIMEOUT_WAITING_FOR_LISTENER, handlerName, |
| | | TimeUnit.MILLISECONDS.toSeconds(LISTEN_TIMEOUT_MS)); |
| | | break; |
| | | } |
| | | waitListen.wait(remainingMs); |
| | | } |
| | | } |
| | | catch (InterruptedException e) |
| | |
| | | |
| | | try |
| | | { |
| | | // At this point, the connection Handler either started correctly or failed |
| | | // to start but the start process should be notified and resume its work in any cases. |
| | | synchronized (waitListen) |
| | | { |
| | | listenAttempted = true; |
| | | waitListen.notifyAll(); |
| | | } |
| | | |
| | | // If we have gotten here, then we are about to start listening |
| | | // for the first time since startup or since we were previously disabled. |
| | | // Start the embedded HTTP server |
| | |
| | | lastIterationFailed = true; |
| | | } |
| | | } |
| | | finally |
| | | { |
| | | // At this point, the connection handler either started correctly or failed to start |
| | | // but the start process should be notified and resume its work in any cases. |
| | | synchronized (waitListen) |
| | | { |
| | | listenAttempted = true; |
| | | waitListen.notifyAll(); |
| | | } |
| | | } |
| | | } |
| | | |
| | | // Initiate shutdown |
| | |
| | | use request controls |
| | | ERR_CONNHANDLER_CANNOT_BIND_432=The %s connection handler \ |
| | | defined in configuration entry %s was unable to bind to %s:%d: %s |
| | | ERR_CONNHANDLER_TIMEOUT_WAITING_FOR_LISTENER_1540=The %s did not attempt to \ |
| | | open its listen port within %d seconds. The Directory Server will stop \ |
| | | waiting for it and continue, but that connection handler may never accept \ |
| | | connections |
| | | ERR_JMX_SEARCH_INSUFFICIENT_PRIVILEGES_438=You do not have sufficient \ |
| | | privileges to perform search operations through JMX |
| | | ERR_JMX_INSUFFICIENT_PRIVILEGES_439=You do not have sufficient \ |
| | |
| | | import java.lang.management.ThreadInfo; |
| | | import java.lang.management.ThreadMXBean; |
| | | import java.net.BindException; |
| | | import java.net.ConnectException; |
| | | import java.net.InetSocketAddress; |
| | | import java.net.ServerSocket; |
| | | import java.net.Socket; |
| | |
| | | } |
| | | |
| | | /** |
| | | * Asserts that the given local port accepts connections right now, without any retry loop. |
| | | * <p> |
| | | * Intended for connection handler tests: a handler start method must not return before its listen |
| | | * port is open. When the port is closed this method tells the two possible causes apart, because a |
| | | * plain connection failure cannot distinguish them: the handler may never have managed to bind at |
| | | * all, or its start method may have returned too early. |
| | | * |
| | | * @param port |
| | | * the port the connection handler under test is supposed to be listening on |
| | | * @throws IOException |
| | | * if the connection fails for a reason other than the port being closed |
| | | */ |
| | | public static void assertPortIsAcceptingConnections(int port) throws IOException |
| | | { |
| | | try |
| | | { |
| | | connectTo(port); |
| | | } |
| | | catch (ConnectException e) |
| | | { |
| | | // Give the handler thread the extra time it should not have needed. If the port opens now, the |
| | | // handler can bind and the start method simply returned too early, which is the regression this |
| | | // check is about. If it never opens, the test failed for an unrelated reason. |
| | | assertTrue(waitForPortToOpen(port), "the connection handler never opened port " + port); |
| | | fail("the connection handler start method returned before port " + port + " was open"); |
| | | } |
| | | } |
| | | |
| | | private static void connectTo(int port) throws IOException |
| | | { |
| | | try (Socket socket = new Socket()) |
| | | { |
| | | socket.connect(new InetSocketAddress("127.0.0.1", port), 1000); |
| | | } |
| | | } |
| | | |
| | | private static boolean waitForPortToOpen(int port) |
| | | { |
| | | final long deadline = System.currentTimeMillis() + 10000; |
| | | do |
| | | { |
| | | try |
| | | { |
| | | connectTo(port); |
| | | return true; |
| | | } |
| | | catch (IOException ignored) |
| | | { |
| | | sleep(100); |
| | | } |
| | | } |
| | | while (System.currentTimeMillis() < deadline); |
| | | return false; |
| | | } |
| | | |
| | | /** |
| | | * Finds a free server socket port on the local host. |
| | | * |
| | | * @return The free port. |
| New file |
| | |
| | | /* |
| | | * The contents of this file are subject to the terms of the Common Development and |
| | | * Distribution License (the License). You may not use this file except in compliance with the |
| | | * License. |
| | | * |
| | | * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the |
| | | * specific language governing permission and limitations under the License. |
| | | * |
| | | * When distributing Covered Software, include this CDDL Header Notice in each file and include |
| | | * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL |
| | | * Header, with the fields enclosed by brackets [] replaced by your own identifying |
| | | * information: "Portions copyright [year] [name of copyright owner]". |
| | | * |
| | | * Copyright 2026 3A Systems, LLC. |
| | | */ |
| | | package org.opends.server.protocols.http; |
| | | |
| | | import static org.testng.Assert.*; |
| | | |
| | | import org.forgerock.i18n.LocalizableMessage; |
| | | import org.forgerock.opendj.server.config.meta.HTTPConnectionHandlerCfgDefn; |
| | | import org.forgerock.opendj.server.config.server.HTTPConnectionHandlerCfg; |
| | | import org.opends.server.DirectoryServerTestCase; |
| | | import org.opends.server.TestCaseUtils; |
| | | import org.opends.server.core.DirectoryServer; |
| | | import org.opends.server.extensions.InitializationUtils; |
| | | import org.opends.server.types.Entry; |
| | | import org.testng.annotations.BeforeClass; |
| | | import org.testng.annotations.Test; |
| | | |
| | | @SuppressWarnings("javadoc") |
| | | @Test(groups = { "precommit", "http" }, sequential = true) |
| | | public class HTTPConnectionHandlerTestCase extends DirectoryServerTestCase |
| | | { |
| | | private static final LocalizableMessage STOP_REASON = LocalizableMessage.raw("Don't need a reason."); |
| | | |
| | | @BeforeClass |
| | | public void setUp() throws Exception |
| | | { |
| | | // This test suite depends on having the schema available, so we'll start the server. |
| | | TestCaseUtils.startServer(); |
| | | } |
| | | |
| | | /** |
| | | * The start method must not return before the handler thread has attempted to start the embedded |
| | | * HTTP server, otherwise a client connecting right after the handler has been enabled through |
| | | * dsconfig can be refused. |
| | | * |
| | | * @throws Exception |
| | | * if the handler cannot be instantiated or started. |
| | | */ |
| | | @Test |
| | | public void testStartWaitsForListenPort() throws Exception |
| | | { |
| | | final int listenPort = TestCaseUtils.findFreePort(); |
| | | Entry handlerEntry = TestCaseUtils.makeEntry( |
| | | "dn: cn=HTTP Connection Handler,cn=Connection Handlers,cn=config", |
| | | "objectClass: top", |
| | | "objectClass: ds-cfg-connection-handler", |
| | | "objectClass: ds-cfg-http-connection-handler", |
| | | "cn: HTTP Connection Handler", |
| | | "ds-cfg-java-class: org.opends.server.protocols.http.HTTPConnectionHandler", |
| | | "ds-cfg-enabled: true", |
| | | "ds-cfg-listen-address: 127.0.0.1", |
| | | "ds-cfg-listen-port: " + listenPort, |
| | | "ds-cfg-accept-backlog: 128", |
| | | "ds-cfg-keep-stats: false", |
| | | "ds-cfg-use-tcp-keep-alive: true", |
| | | "ds-cfg-use-tcp-no-delay: true", |
| | | "ds-cfg-allow-tcp-reuse-address: true", |
| | | "ds-cfg-max-request-size: 5 megabytes", |
| | | "ds-cfg-buffer-size: 4096 bytes", |
| | | "ds-cfg-max-blocked-write-time-limit: 2 minutes", |
| | | "ds-cfg-use-ssl: false", |
| | | "ds-cfg-ssl-client-auth-policy: optional", |
| | | "ds-cfg-ssl-cert-nickname: server-cert"); |
| | | HTTPConnectionHandlerCfg config = |
| | | InitializationUtils.getConfiguration(HTTPConnectionHandlerCfgDefn.getInstance(), handlerEntry); |
| | | |
| | | HTTPConnectionHandler handler = new HTTPConnectionHandler(); |
| | | handler.initializeConnectionHandler(DirectoryServer.getInstance().getServerContext(), config); |
| | | try |
| | | { |
| | | handler.start(); |
| | | |
| | | // No retry loop here on purpose: once start() has returned, the port must already be open. |
| | | TestCaseUtils.assertPortIsAcceptingConnections(listenPort); |
| | | } |
| | | finally |
| | | { |
| | | handler.processServerShutdown(STOP_REASON); |
| | | handler.finalizeConnectionHandler(STOP_REASON); |
| | | handler.join(10000); |
| | | assertFalse(handler.isAlive(), "the connection handler thread is still running"); |
| | | } |
| | | } |
| | | } |
| | |
| | | * |
| | | * Copyright 2006-2009 Sun Microsystems, Inc. |
| | | * Portions Copyright 2012-2016 ForgeRock AS. |
| | | * Portions Copyright 2025 3A Systems, LLC. |
| | | * Portions Copyright 2025-2026 3A Systems, LLC. |
| | | */ |
| | | package org.opends.server.protocols.ldap; |
| | | |
| | |
| | | } |
| | | |
| | | /** |
| | | * The start method must not return before the handler thread has attempted to |
| | | * open the listen port, otherwise a client connecting right after the server |
| | | * startup can be refused. |
| | | * |
| | | * @throws Exception if the handler cannot be instantiated or started. |
| | | */ |
| | | @Test |
| | | public void testStartWaitsForListenPort() throws Exception { |
| | | Entry handlerEntry = TestCaseUtils.makeEntry( |
| | | "dn: cn=LDAP Connection Handler,cn=Connection Handlers,cn=config", |
| | | "objectClass: top", |
| | | "objectClass: ds-cfg-connection-handler", |
| | | "objectClass: ds-cfg-ldap-connection-handler", |
| | | "cn: LDAP Connection Handler", |
| | | "ds-cfg-java-class: org.forgerock.opendj.reactive.LDAPConnectionHandler2", |
| | | "ds-cfg-enabled: true", |
| | | "ds-cfg-listen-address: 127.0.0.1", |
| | | "ds-cfg-accept-backlog: 128", |
| | | "ds-cfg-allow-ldap-v2: false", |
| | | "ds-cfg-keep-stats: false", |
| | | "ds-cfg-use-tcp-keep-alive: true", |
| | | "ds-cfg-use-tcp-no-delay: true", |
| | | "ds-cfg-allow-tcp-reuse-address: true", |
| | | "ds-cfg-send-rejection-notice: true", |
| | | "ds-cfg-max-request-size: 5 megabytes", |
| | | "ds-cfg-num-request-handlers: 2", |
| | | "ds-cfg-allow-start-tls: false", |
| | | "ds-cfg-use-ssl: false", |
| | | "ds-cfg-ssl-client-auth-policy: optional", |
| | | "ds-cfg-ssl-cert-nickname: server-cert", |
| | | "ds-cfg-key-manager-provider: cn=JKS,cn=Key Manager Providers,cn=config", |
| | | "ds-cfg-trust-manager-provider: cn=JKS,cn=Trust Manager Providers,cn=config"); |
| | | LDAPConnectionHandler2 handler = getLDAPHandlerInstance(handlerEntry); |
| | | int listenPort = handler.getListeners().iterator().next().getPort(); |
| | | try |
| | | { |
| | | handler.start(); |
| | | |
| | | // No retry loop here on purpose: once start() has returned, the port must already be open. |
| | | TestCaseUtils.assertPortIsAcceptingConnections(listenPort); |
| | | } |
| | | finally |
| | | { |
| | | handler.processServerShutdown(reasonMsg); |
| | | handler.finalizeConnectionHandler(reasonMsg); |
| | | handler.join(10000); |
| | | assertFalse(handler.isAlive(), "the connection handler thread is still running"); |
| | | } |
| | | } |
| | | |
| | | /** |
| | | * Start a handler an then give its hasAcceptableConfiguration a Entry with |
| | | * numerous invalid cases and single-valued attrs with duplicate values. |
| | | * |