[#794] Wait for the listen port to be open before returning from connection handler start

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;
 
@@ -679,6 +686,38 @@
         logger.info(NOTE_CONNHANDLER_STARTED_LISTENING, handlerName);
     }
 
+    /**
+     * {@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();
+                }
             }
         }
 

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 @@ public abstract class ConnectionHandler
 
   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;
 

opendj-server-legacy/src/main/java/org/opends/server/protocols/http/HTTPConnectionHandler.java

@@ -167,8 +167,10 @@ public class HTTPConnectionHandler extends ConnectionHandler<HTTPConnectionHandl
   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 @@ private boolean isListening()
     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 @@ 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)
@@ -654,14 +672,6 @@ public void run()
 
       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 @@ public void run()
           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

opendj-server-legacy/src/messages/org/opends/messages/protocol.properties

@@ -729,6 +729,10 @@ ERR_LDAPV2_CONTROLS_NOT_ALLOWED_431=LDAPv2 clients are not allowed to \
  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 \

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;
@@ -841,6 +842,62 @@ public static int[] findFreePorts(int nb) throws IOException
     }
   }
 
+  /**
+   * 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.
    *

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");
+    }
+  }
+}