mirror of https://github.com/OpenIdentityPlatform/OpenDJ.git

Valery Kharseko
22 hours ago a2c7a1aafbd30c89e1c857ec8574080dcd83aa52
[#794] Wait for the listen port to be open before returning from connection handler start (#796)
6 files modified
1 files added
322 ■■■■■ changed files
opendj-server-legacy/src/main/java/org/forgerock/opendj/reactive/LDAPConnectionHandler2.java 53 ●●●● patch | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/api/ConnectionHandler.java 16 ●●●●● patch | view | raw | blame | history
opendj-server-legacy/src/main/java/org/opends/server/protocols/http/HTTPConnectionHandler.java 42 ●●●● patch | view | raw | blame | history
opendj-server-legacy/src/messages/org/opends/messages/protocol.properties 4 ●●●● patch | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/TestCaseUtils.java 57 ●●●●● patch | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/protocols/http/HTTPConnectionHandlerTestCase.java 97 ●●●●● patch | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/protocols/ldap/TestLDAPConnectionHandler.java 53 ●●●●● patch | view | raw | blame | history
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();
                }
            }
        }
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;
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
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 \
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.
opendj-server-legacy/src/test/java/org/opends/server/protocols/http/HTTPConnectionHandlerTestCase.java
New file
@@ -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");
    }
  }
}
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.
   *