From a2c7a1aafbd30c89e1c857ec8574080dcd83aa52 Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Fri, 31 Jul 2026 17:01:00 +0000
Subject: [PATCH] [#794] Wait for the listen port to be open before returning from connection handler start (#796)
---
opendj-server-legacy/src/main/java/org/opends/server/api/ConnectionHandler.java | 16 ++
opendj-server-legacy/src/test/java/org/opends/server/TestCaseUtils.java | 57 +++++++++
opendj-server-legacy/src/test/java/org/opends/server/protocols/ldap/TestLDAPConnectionHandler.java | 53 ++++++++
opendj-server-legacy/src/test/java/org/opends/server/protocols/http/HTTPConnectionHandlerTestCase.java | 97 ++++++++++++++++
opendj-server-legacy/src/messages/org/opends/messages/protocol.properties | 4
opendj-server-legacy/src/main/java/org/forgerock/opendj/reactive/LDAPConnectionHandler2.java | 53 +++++++-
opendj-server-legacy/src/main/java/org/opends/server/protocols/http/HTTPConnectionHandler.java | 42 +++++-
7 files changed, 304 insertions(+), 18 deletions(-)
diff --git a/opendj-server-legacy/src/main/java/org/forgerock/opendj/reactive/LDAPConnectionHandler2.java b/opendj-server-legacy/src/main/java/org/forgerock/opendj/reactive/LDAPConnectionHandler2.java
index 27581c4..482b318 100644
--- a/opendj-server-legacy/src/main/java/org/forgerock/opendj/reactive/LDAPConnectionHandler2.java
+++ b/opendj-server-legacy/src/main/java/org/forgerock/opendj/reactive/LDAPConnectionHandler2.java
@@ -190,6 +190,13 @@
*/
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;
@@ -680,6 +687,38 @@
}
/**
+ * {@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.
*/
@@ -702,6 +741,7 @@
// so notify here to allow the server startup to complete.
synchronized (waitListen) {
starting = false;
+ listenAttempted = true;
waitListen.notifyAll();
}
}
@@ -717,12 +757,6 @@
}
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();
@@ -750,6 +784,13 @@
} 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();
+ }
}
}
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/api/ConnectionHandler.java b/opendj-server-legacy/src/main/java/org/opends/server/api/ConnectionHandler.java
index 6a33bb3..3645b8b 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/api/ConnectionHandler.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/api/ConnectionHandler.java
@@ -13,6 +13,7 @@
*
* Copyright 2006-2009 Sun Microsystems, Inc.
* Portions Copyright 2012-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.server.api;
@@ -23,6 +24,7 @@
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;
@@ -53,6 +55,20 @@
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;
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/protocols/http/HTTPConnectionHandler.java b/opendj-server-legacy/src/main/java/org/opends/server/protocols/http/HTTPConnectionHandler.java
index af5b881..7f9cce5 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/protocols/http/HTTPConnectionHandler.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/protocols/http/HTTPConnectionHandler.java
@@ -167,8 +167,10 @@
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;
@@ -574,6 +576,14 @@
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()
{
@@ -584,11 +594,19 @@
{
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)
@@ -654,14 +672,6 @@
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
@@ -695,6 +705,16 @@
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
diff --git a/opendj-server-legacy/src/messages/org/opends/messages/protocol.properties b/opendj-server-legacy/src/messages/org/opends/messages/protocol.properties
index 745726d..60dccc8 100644
--- a/opendj-server-legacy/src/messages/org/opends/messages/protocol.properties
+++ b/opendj-server-legacy/src/messages/org/opends/messages/protocol.properties
@@ -729,6 +729,10 @@
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 \
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/TestCaseUtils.java b/opendj-server-legacy/src/test/java/org/opends/server/TestCaseUtils.java
index 9f220fa..fdd3d63 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/TestCaseUtils.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/TestCaseUtils.java
@@ -51,6 +51,7 @@
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;
@@ -842,6 +843,62 @@
}
/**
+ * 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.
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/protocols/http/HTTPConnectionHandlerTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/protocols/http/HTTPConnectionHandlerTestCase.java
new file mode 100644
index 0000000..456284b
--- /dev/null
+++ b/opendj-server-legacy/src/test/java/org/opends/server/protocols/http/HTTPConnectionHandlerTestCase.java
@@ -0,0 +1,97 @@
+/*
+ * 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");
+ }
+ }
+}
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/protocols/ldap/TestLDAPConnectionHandler.java b/opendj-server-legacy/src/test/java/org/opends/server/protocols/ldap/TestLDAPConnectionHandler.java
index c0a6467..5d16745 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/protocols/ldap/TestLDAPConnectionHandler.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/protocols/ldap/TestLDAPConnectionHandler.java
@@ -13,7 +13,7 @@
*
* 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;
@@ -127,6 +127,57 @@
}
/**
+ * 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.
*
--
Gitblit v1.10.0