From ebb19c5816399b83c42b8f9194dbf3d9296975a3 Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Thu, 30 Jul 2026 07:45:56 +0000
Subject: [PATCH] Fix concurrency and equals-contract CodeQL alerts (#785)

---
 opendj-server-legacy/src/main/java/org/opends/server/util/cli/PointAdder.java                       |   31 ++
 opendj-server-legacy/src/test/java/org/opends/guitools/controlpanel/datamodel/ScheduleTypeTest.java |   77 ++++++++
 opendj-server-legacy/src/main/java/org/opends/server/core/DirectoryServer.java                      |   74 +++++--
 opendj-server-legacy/src/main/java/org/opends/quicksetup/Application.java                           |   30 ++
 opendj-server-legacy/src/main/java/org/opends/admin/ads/SuffixDescriptor.java                       |   26 ++
 opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/datamodel/ScheduleType.java     |    3 
 opendj-server-legacy/src/main/java/org/opends/server/plugins/profiler/ProfileStack.java             |   50 +---
 opendj-server-legacy/src/test/java/org/opends/admin/ads/SuffixDescriptorTest.java                   |  132 ++++++++++++++
 opendj-server-legacy/src/main/java/org/opends/server/loggers/DebugLogPublisher.java                 |   37 +++-
 opendj-server-legacy/src/main/java/org/opends/server/plugins/profiler/ProfileStackFrame.java        |   28 --
 10 files changed, 379 insertions(+), 109 deletions(-)

diff --git a/opendj-server-legacy/src/main/java/org/opends/admin/ads/SuffixDescriptor.java b/opendj-server-legacy/src/main/java/org/opends/admin/ads/SuffixDescriptor.java
index 8c61c45..9d87a52 100644
--- a/opendj-server-legacy/src/main/java/org/opends/admin/ads/SuffixDescriptor.java
+++ b/opendj-server-legacy/src/main/java/org/opends/admin/ads/SuffixDescriptor.java
@@ -13,6 +13,7 @@
  *
  * Copyright 2008 Sun Microsystems, Inc.
  * Portions Copyright 2015-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
  */
 package org.opends.admin.ads;
 
@@ -107,6 +108,13 @@
   }
 
   @Override
+  public boolean equals(Object o)
+  {
+    return this == o
+        || (o instanceof SuffixDescriptor && getId().equals(((SuffixDescriptor) o).getId()));
+  }
+
+  @Override
   public int hashCode()
   {
     return getId().hashCode();
@@ -114,16 +122,28 @@
 
   /**
    * Returns an Id that is unique for this suffix.
+   * <p>
+   * The id is built from the suffix DN and the ids of the servers holding a replica of it.
+   * The server ids are sorted because {@link #getReplicas()} hands out a {@link HashSet} of
+   * {@link ReplicaDescriptor}s, which do not override {@code hashCode()}: without the sort,
+   * two descriptors describing the same suffix on the same servers could produce different
+   * ids depending on the iteration order.
    *
    * @return an Id that is unique for this suffix.
    */
   public String getId()
   {
-    StringBuilder buf = new StringBuilder();
-    buf.append(getDN());
+    Set<String> serverIds = new TreeSet<>();
     for (ReplicaDescriptor replica : getReplicas())
     {
-      buf.append("-").append(replica.getServer().getId());
+      serverIds.add(replica.getServer().getId());
+    }
+
+    StringBuilder buf = new StringBuilder();
+    buf.append(getDN());
+    for (String serverId : serverIds)
+    {
+      buf.append("-").append(serverId);
     }
     return buf.toString();
   }
diff --git a/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/datamodel/ScheduleType.java b/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/datamodel/ScheduleType.java
index cb6eed8..92963d3 100644
--- a/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/datamodel/ScheduleType.java
+++ b/opendj-server-legacy/src/main/java/org/opends/guitools/controlpanel/datamodel/ScheduleType.java
@@ -13,6 +13,7 @@
  *
  * Copyright 2009 Sun Microsystems, Inc.
  * Portions Copyright 2015-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
  */
 package org.opends.guitools.controlpanel.datamodel;
 
@@ -119,7 +120,7 @@
     {
       return true;
     }
-    return o != null
+    return o instanceof ScheduleType
         && toString().equals(o.toString());
   }
 
diff --git a/opendj-server-legacy/src/main/java/org/opends/quicksetup/Application.java b/opendj-server-legacy/src/main/java/org/opends/quicksetup/Application.java
index 53bcf6a..18a3f40 100644
--- a/opendj-server-legacy/src/main/java/org/opends/quicksetup/Application.java
+++ b/opendj-server-legacy/src/main/java/org/opends/quicksetup/Application.java
@@ -13,6 +13,7 @@
  *
  * Copyright 2008-2010 Sun Microsystems, Inc.
  * Portions Copyright 2012-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
  */
 package org.opends.quicksetup;
 
@@ -799,9 +800,12 @@
   /** Class used to add points periodically to the end of the logs. */
   protected class PointAdder implements Runnable
   {
-    private Thread t;
-    private boolean stopPointAdder;
-    private boolean pointAdderStopped;
+    /** Written by start() and read by stop(), hence volatile. */
+    private volatile Thread t;
+    /** Written by stop() and read by the point adder thread, hence volatile. */
+    private volatile boolean stopPointAdder;
+    /** Written by the point adder thread and read by stop(), hence volatile. */
+    private volatile boolean pointAdderStopped;
 
     /** Default constructor. */
     public PointAdder()
@@ -825,21 +829,31 @@
       t.start();
     }
 
-    /** Stops the PointAdder: points are no longer added at the end of the logs periodically. */
-    public synchronized void stop()
+    /**
+     * Stops the PointAdder: points are no longer added at the end of the logs periodically.
+     * <p>
+     * Calling this method on a PointAdder that was never started is a no-op.
+     */
+    public void stop()
     {
       stopPointAdder = true;
+      final Thread thread = t;
+      if (thread == null)
+      {
+        return;
+      }
       while (!pointAdderStopped)
       {
+        thread.interrupt();
         try
         {
-          t.interrupt();
           // To allow the thread to set the boolean.
           Thread.sleep(100);
         }
-        catch (Throwable t)
+        catch (InterruptedException e)
         {
-          // do nothing
+          Thread.currentThread().interrupt();
+          break;
         }
       }
     }
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/core/DirectoryServer.java b/opendj-server-legacy/src/main/java/org/opends/server/core/DirectoryServer.java
index e5eeb61..9578799 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/core/DirectoryServer.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/core/DirectoryServer.java
@@ -13,7 +13,7 @@
  *
  * Copyright 2006-2010 Sun Microsystems, Inc.
  * Portions Copyright 2010-2016 ForgeRock AS.
- * Portions Copyright 2022-2025 3A Systems, LLC.
+ * Portions Copyright 2022-2026 3A Systems, LLC.
  * Portions Copyright 2025 Wren Security.
  */
 package org.opends.server.core;
@@ -177,11 +177,27 @@
 {
   private static final LocalizedLogger logger = LocalizedLogger.getLoggerForThisClass();
 
-  /** The singleton Directory Server instance. */
-  private static DirectoryServer directoryServer = new DirectoryServer();
+  /**
+   * Guards the life cycle of the {@link #directoryServer} singleton: bootstrapping,
+   * starting, shutting down, and replacing the instance during an in-core restart.
+   * A dedicated lock is used because the {@code directoryServer} field is reassigned
+   * by {@link #getNewInstance(DirectoryEnvironmentConfig)}: synchronizing on the field
+   * itself would let threads acquire different monitors and provide no exclusion at all.
+   * <p>
+   * The lock makes the guarded transitions atomic; it does not make the life cycle flags
+   * visible to the code that reads them without holding it (see {@link #isRunning()} and
+   * {@link #isShuttingDown()}), which is why those flags are {@code volatile}.
+   */
+  private static final Object LIFECYCLE_LOCK = new Object();
 
-  /** Indicates whether the server currently holds an exclusive lock on the server lock file. */
-  private static boolean serverLocked;
+  /** The singleton Directory Server instance. */
+  private static volatile DirectoryServer directoryServer = new DirectoryServer();
+
+  /**
+   * Indicates whether the server currently holds an exclusive lock on the server lock file.
+   * Written outside {@link #LIFECYCLE_LOCK} on shutdown, hence volatile.
+   */
+  private static volatile boolean serverLocked;
 
   /** The message to be displayed on the command-line when the user asks for the usage. */
   private static final LocalizableMessage toolDescription = INFO_DSCORE_TOOL_DESCRIPTION.get();
@@ -228,15 +244,18 @@
   /** The configuration manager that will handle the server backends. */
   private BackendConfigManager backendConfigManager;
 
-  /** Indicates whether the server has been bootstrapped. */
-  private boolean isBootstrapped;
-  /** Indicates whether the server is currently online. */
-  private boolean isRunning;
+  /** Indicates whether the server has been bootstrapped. Read without holding {@link #LIFECYCLE_LOCK}. */
+  private volatile boolean isBootstrapped;
+  /** Indicates whether the server is currently online. Read without holding {@link #LIFECYCLE_LOCK}. */
+  private volatile boolean isRunning;
   /** Indicates whether the server is currently in "lockdown mode". */
   private boolean lockdownMode;
 
-  /** Indicates whether the server is currently in the process of shutting down. */
-  private boolean shuttingDown;
+  /**
+   * Indicates whether the server is currently in the process of shutting down.
+   * Read without holding {@link #LIFECYCLE_LOCK}.
+   */
+  private volatile boolean shuttingDown;
 
   /** The configuration manager that will handle the certificate mapper. */
   private CertificateMapperConfigManager certificateMapperConfigManager;
@@ -284,7 +303,7 @@
    * a mapping between the DN of the associated configuration entry and the
    * policy implementation.
    */
-  private ConcurrentMap<DN, AuthenticationPolicy> authenticationPolicies;
+  private final ConcurrentMap<DN, AuthenticationPolicy> authenticationPolicies = new ConcurrentHashMap<>();
 
   /**
    * The set of password validators registered with the Directory Server, as a
@@ -356,7 +375,7 @@
   /** The set of alert handlers registered with the Directory Server. */
   private List<AlertHandler<?>> alertHandlers;
   /** The set of connection handlers registered with the Directory Server. */
-  private List<ConnectionHandler<?>> connectionHandlers;
+  private final List<ConnectionHandler<?>> connectionHandlers = new CopyOnWriteArrayList<>();
 
   /** The set of backup task listeners registered with the Directory Server. */
   private CopyOnWriteArrayList<BackupTaskListener> backupTaskListeners;
@@ -430,7 +449,7 @@
   private KeyManagerProviderConfigManager keyManagerProviderConfigManager;
 
   /** The set of connections that are currently established. */
-  private Set<ClientConnection> establishedConnections;
+  private final Set<ClientConnection> establishedConnections = new LinkedHashSet<>(1000);
 
   /** The log rotation policy config manager for the Directory Server. */
   private LogRotationPolicyConfigManager rotationPolicyConfigManager;
@@ -1059,7 +1078,7 @@
   private static DirectoryServer
                       getNewInstance(DirectoryEnvironmentConfig config)
   {
-    synchronized (directoryServer)
+    synchronized (LIFECYCLE_LOCK)
     {
       return directoryServer = new DirectoryServer(config);
     }
@@ -1122,7 +1141,7 @@
    */
   public static void bootstrapClient()
   {
-    synchronized (directoryServer)
+    synchronized (LIFECYCLE_LOCK)
     {
       // Schema handler contains a default schema to start with
       directoryServer.schemaHandler = new SchemaHandler();
@@ -1143,14 +1162,14 @@
       directoryServer.rotationPolicies = new ConcurrentHashMap<>();
       directoryServer.retentionPolicies = new ConcurrentHashMap<>();
       directoryServer.certificateMappers = new ConcurrentHashMap<>();
-      directoryServer.authenticationPolicies = new ConcurrentHashMap<>();
+      directoryServer.authenticationPolicies.clear();
       directoryServer.defaultPasswordPolicy = null;
       directoryServer.monitorProviders = new ConcurrentHashMap<>();
       directoryServer.initializationCompletedListeners = new CopyOnWriteArrayList<>();
       directoryServer.shutdownListeners = new CopyOnWriteArrayList<>();
       directoryServer.synchronizationProviders = new CopyOnWriteArrayList<>();
       directoryServer.supportedLDAPVersions = new ConcurrentHashMap<>();
-      directoryServer.connectionHandlers = new CopyOnWriteArrayList<>();
+      directoryServer.connectionHandlers.clear();
       directoryServer.identityMappers = new ConcurrentHashMap<>();
       directoryServer.extendedOperationHandlers = new ConcurrentHashMap<>();
       directoryServer.saslMechanismHandlers = new ConcurrentHashMap<>();
@@ -1180,7 +1199,7 @@
     // First, make sure that the server isn't currently running.  If it isn't,
     // then make sure that no other thread will try to start or bootstrap the
     // server before this thread is done.
-    synchronized (directoryServer)
+    synchronized (LIFECYCLE_LOCK)
     {
       if (isRunning)
       {
@@ -1205,7 +1224,10 @@
     bootstrapClient();
 
     // Initialize the variables that will be used for connection tracking.
-    establishedConnections = new LinkedHashSet<>(1000);
+    synchronized (establishedConnections)
+    {
+      establishedConnections.clear();
+    }
     currentConnections     = 0;
     maxConnections         = 0;
     totalConnections       = 0;
@@ -1216,7 +1238,7 @@
     pluginConfigManager = new PluginConfigManager(serverContext);
 
     // If we have gotten here, then the configuration should be properly bootstrapped.
-    synchronized (directoryServer)
+    synchronized (LIFECYCLE_LOCK)
     {
       isBootstrapped = true;
     }
@@ -1331,7 +1353,7 @@
       throw new InitializationException(e.getMessageObject());
     }
 
-    synchronized (directoryServer)
+    synchronized (LIFECYCLE_LOCK)
     {
       if (! isBootstrapped)
       {
@@ -2758,7 +2780,7 @@
    */
   public static void setEntryCache(EntryCache<?> entryCache)
   {
-    synchronized (directoryServer)
+    synchronized (LIFECYCLE_LOCK)
     {
       directoryServer.entryCache = entryCache;
     }
@@ -4084,7 +4106,7 @@
    */
   public static void shutDown(String className, LocalizableMessage reason)
   {
-    synchronized (directoryServer)
+    synchronized (LIFECYCLE_LOCK)
     {
       if (directoryServer.shuttingDown)
       {
@@ -4319,7 +4341,9 @@
     // doing that, destroy the previous instance.
     DirectoryEnvironmentConfig envConfig = directoryServer.environmentConfig;
     directoryServer.destroy();
-    directoryServer = getNewInstance(envConfig);
+    // getNewInstance() publishes the new instance under LIFECYCLE_LOCK: assigning its
+    // result here again would repeat that write outside the lock.
+    getNewInstance(envConfig);
   }
 
   /**
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/loggers/DebugLogPublisher.java b/opendj-server-legacy/src/main/java/org/opends/server/loggers/DebugLogPublisher.java
index 810db5c..7d8489a 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/loggers/DebugLogPublisher.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/loggers/DebugLogPublisher.java
@@ -13,13 +13,13 @@
  *
  * Copyright 2009 Sun Microsystems, Inc.
  * Portions Copyright 2012-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
  */
 package org.opends.server.loggers;
 
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.TreeMap;
+import java.util.concurrent.ConcurrentHashMap;
 
 import org.forgerock.i18n.LocalizableMessage;
 import org.forgerock.opendj.server.config.server.DebugLogPublisherCfg;
@@ -42,11 +42,24 @@
   /** The default global settings key. */
   private static final String GLOBAL= "_global";
 
-  /** The map of class names to their trace settings. */
-  private Map<String,TraceSettings> classTraceSettings;
+  /**
+   * The map of class names to their trace settings.
+   * <p>
+   * The getters below read this map without any lock, so the field is volatile
+   * (the map is created lazily and must be published safely) and the map itself
+   * is concurrent; all writes go through the synchronized mutators below.
+   */
+  private volatile Map<String,TraceSettings> classTraceSettings;
 
-  /** The map of class names to their method trace settings. */
-  private Map<String,Map<String,TraceSettings>> methodTraceSettings;
+  /**
+   * The map of class names to their method trace settings.
+   * <p>
+   * Concurrent for the same reason as {@link #classTraceSettings}. The nested maps
+   * are concurrent as well: {@link DebugTracer} keeps the one returned by
+   * {@link #getMethodSettings(String)} and iterates it while another thread may be
+   * reconfiguring the publisher.
+   */
+  private volatile Map<String,Map<String,TraceSettings>> methodTraceSettings;
 
 
 
@@ -146,6 +159,10 @@
    *                   {@code null} to set the trace settings for the
    *                   global scope.
    * @param  settings  The trace settings for the specified scope.
+   *                   Must not be {@code null}.
+   * @throws NullPointerException  If {@code settings} is {@code null}: the trace
+   *                   settings are held in concurrent maps, which do not accept
+   *                   null values.
    */
   public final void addTraceSettings(String scope, TraceSettings settings)
   {
@@ -213,7 +230,7 @@
    *          {@code null} if no trace setting is defined for that
    *          scope.
    */
-  final TraceSettings removeTraceSettings(String scope)
+  final synchronized TraceSettings removeTraceSettings(String scope)
   {
     TraceSettings removedSettings = null;
     if (scope == null) {
@@ -262,7 +279,7 @@
   {
     if (classTraceSettings == null)
     {
-      classTraceSettings = new HashMap<>();
+      classTraceSettings = new ConcurrentHashMap<>();
     }
     classTraceSettings.put(className, settings);
   }
@@ -280,12 +297,12 @@
       String methodName, TraceSettings settings)
   {
     if (methodTraceSettings == null) {
-      methodTraceSettings = new HashMap<>();
+      methodTraceSettings = new ConcurrentHashMap<>();
     }
     Map<String, TraceSettings> methodLevels = methodTraceSettings.get(className);
     if (methodLevels == null)
     {
-      methodLevels = new TreeMap<>();
+      methodLevels = new ConcurrentHashMap<>();
       methodTraceSettings.put(className, methodLevels);
     }
     methodLevels.put(methodName, settings);
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/plugins/profiler/ProfileStack.java b/opendj-server-legacy/src/main/java/org/opends/server/plugins/profiler/ProfileStack.java
index 70129c1..29d4cfa 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/plugins/profiler/ProfileStack.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/plugins/profiler/ProfileStack.java
@@ -13,12 +13,12 @@
  *
  * Copyright 2006-2008 Sun Microsystems, Inc.
  * Portions Copyright 2014-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
  */
 package org.opends.server.plugins.profiler;
 
 import java.io.IOException;
 
-import org.forgerock.i18n.slf4j.LocalizedLogger;
 import org.forgerock.opendj.io.ASN1Reader;
 import org.forgerock.opendj.io.ASN1Writer;
 
@@ -28,11 +28,6 @@
  */
 public class ProfileStack
 {
-  private static final LocalizedLogger logger = LocalizedLogger.getLoggerForThisClass();
-
-
-
-
   /**
    * The line number that will be used for stack frames in which the line number
    * is unknown but it is not a native method.
@@ -240,43 +235,32 @@
   @Override
   public boolean equals(Object o)
   {
-    if (o == null)
-    {
-      return false;
-    }
-    else if (this == o)
+    if (this == o)
     {
       return true;
     }
-
-
-    try
+    if (!(o instanceof ProfileStack))
     {
-      ProfileStack s = (ProfileStack) o;
+      return false;
+    }
 
-      if (numFrames != s.numFrames)
+    ProfileStack s = (ProfileStack) o;
+    if (numFrames != s.numFrames)
+    {
+      return false;
+    }
+
+    for (int i=0; i < numFrames; i++)
+    {
+      if (lineNumbers[i] != s.lineNumbers[i] ||
+          !classNames[i].equals(s.classNames[i]) ||
+          !methodNames[i].equals(s.methodNames[i]))
       {
         return false;
       }
-
-      for (int i=0; i < numFrames; i++)
-      {
-        if (lineNumbers[i] != s.lineNumbers[i] ||
-            !classNames[i].equals(s.classNames[i]) ||
-            !methodNames[i].equals(s.methodNames[i]))
-        {
-          return false;
-        }
-      }
-
-      return true;
     }
-    catch (Exception e)
-    {
-      logger.traceException(e);
 
-      return false;
-    }
+    return true;
   }
 
 
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/plugins/profiler/ProfileStackFrame.java b/opendj-server-legacy/src/main/java/org/opends/server/plugins/profiler/ProfileStackFrame.java
index 910aff7..ea6f204 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/plugins/profiler/ProfileStackFrame.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/plugins/profiler/ProfileStackFrame.java
@@ -13,6 +13,7 @@
  *
  * Copyright 2006-2008 Sun Microsystems, Inc.
  * Portions Copyright 2014-2015 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
  */
 package org.opends.server.plugins.profiler;
 
@@ -21,9 +22,6 @@
 import java.util.Arrays;
 import java.util.HashMap;
 
-import org.forgerock.i18n.slf4j.LocalizedLogger;
-
-
 /**
  * This class defines a data structure for holding information about a stack
  * frame captured by the Directory Server profiler.  It will contain the class
@@ -34,11 +32,6 @@
 public class ProfileStackFrame
        implements Comparable
 {
-  private static final LocalizedLogger logger = LocalizedLogger.getLoggerForThisClass();
-
-
-
-
   /**
    * The mapping between the line numbers for this stack frame and the
    * number of times that they were encountered.
@@ -302,26 +295,17 @@
   @Override
   public boolean equals(Object o)
   {
-    if (o == null)
-    {
-      return false;
-    }
-    else if (this == o)
+    if (this == o)
     {
       return true;
     }
-
-    try
+    if (!(o instanceof ProfileStackFrame))
     {
-      ProfileStackFrame f = (ProfileStackFrame) o;
-      return className.equals(f.className) && methodName.equals(f.methodName);
-    }
-    catch (Exception e)
-    {
-      logger.traceException(e);
-
       return false;
     }
+
+    ProfileStackFrame f = (ProfileStackFrame) o;
+    return className.equals(f.className) && methodName.equals(f.methodName);
   }
 
 
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/util/cli/PointAdder.java b/opendj-server-legacy/src/main/java/org/opends/server/util/cli/PointAdder.java
index ff10a47..e76b6a8 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/util/cli/PointAdder.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/util/cli/PointAdder.java
@@ -13,6 +13,7 @@
  *
  * Copyright 2010 Sun Microsystems, Inc.
  * Portions Copyright 2014-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
  */
 package org.opends.server.util.cli;
 
@@ -26,9 +27,12 @@
 public class PointAdder implements Runnable
 {
   private final ConsoleApplication app;
-  private Thread t;
-  private boolean stopPointAdder;
-  private boolean pointAdderStopped;
+  /** Written by start() and read by stop(), hence volatile. */
+  private volatile Thread t;
+  /** Written by stop() and read by the point adder thread, hence volatile. */
+  private volatile boolean stopPointAdder;
+  /** Written by the point adder thread and read by stop(), hence volatile. */
+  private volatile boolean pointAdderStopped;
   private final long periodTime;
   private final ProgressMessageFormatter formatter;
 
@@ -78,20 +82,33 @@
     t.start();
   }
 
-  /** Stops the PointAdder: points are no longer added at the end of the logs periodically. */
-  public synchronized void stop()
+  /**
+   * Stops the PointAdder: points are no longer added at the end of the logs periodically.
+   * <p>
+   * Calling this method on a PointAdder that was never started is a no-op: callers commonly
+   * create the PointAdder before the code path that starts it and stop it from a
+   * {@code finally} block.
+   */
+  public void stop()
   {
     stopPointAdder = true;
+    final Thread thread = t;
+    if (thread == null)
+    {
+      return;
+    }
     while (!pointAdderStopped)
     {
+      thread.interrupt();
       try
       {
-        t.interrupt();
         // To allow the thread to set the boolean.
         Thread.sleep(100);
       }
-      catch (Throwable t)
+      catch (InterruptedException e)
       {
+        Thread.currentThread().interrupt();
+        break;
       }
     }
   }
diff --git a/opendj-server-legacy/src/test/java/org/opends/admin/ads/SuffixDescriptorTest.java b/opendj-server-legacy/src/test/java/org/opends/admin/ads/SuffixDescriptorTest.java
new file mode 100644
index 0000000..dc0cf1d
--- /dev/null
+++ b/opendj-server-legacy/src/test/java/org/opends/admin/ads/SuffixDescriptorTest.java
@@ -0,0 +1,132 @@
+/*
+ * 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.admin.ads;
+
+import static org.assertj.core.api.Assertions.*;
+
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+import org.forgerock.opendj.ldap.DN;
+import org.opends.server.DirectoryServerTestCase;
+import org.testng.annotations.Test;
+
+/** Tests the id-based {@code equals()}/{@code hashCode()} contract of {@link SuffixDescriptor}. */
+@Test(groups = { "precommit", "admin" }, sequential = true)
+@SuppressWarnings("javadoc")
+public class SuffixDescriptorTest extends DirectoryServerTestCase
+{
+  private static final DN SUFFIX_DN = DN.valueOf("dc=example,dc=com");
+  private static final DN OTHER_SUFFIX_DN = DN.valueOf("dc=example,dc=org");
+
+  @Test
+  public void suffixesWithTheSameIdAreEqual()
+  {
+    ServerDescriptor server1 = server("server1.example.com");
+    ServerDescriptor server2 = server("server2.example.com");
+
+    SuffixDescriptor suffix1 = suffix(SUFFIX_DN, server1, server2);
+    SuffixDescriptor suffix2 = suffix(SUFFIX_DN, server1, server2);
+
+    assertThat(suffix1.getId()).isEqualTo(suffix2.getId());
+    assertThat(suffix1).isEqualTo(suffix2);
+    assertThat(suffix1.hashCode()).isEqualTo(suffix2.hashCode());
+    assertThat(suffix1.compareTo(suffix2)).isZero();
+  }
+
+  @Test
+  public void suffixesWithTheSameIdCollapseInASet()
+  {
+    ServerDescriptor server = server("server1.example.com");
+
+    Set<SuffixDescriptor> suffixes = new HashSet<>();
+    suffixes.add(suffix(SUFFIX_DN, server));
+    suffixes.add(suffix(SUFFIX_DN, server));
+
+    assertThat(suffixes).hasSize(1);
+  }
+
+  @Test
+  public void suffixesOnDifferentDnsAreNotEqual()
+  {
+    ServerDescriptor server = server("server1.example.com");
+
+    assertThat(suffix(SUFFIX_DN, server)).isNotEqualTo(suffix(OTHER_SUFFIX_DN, server));
+  }
+
+  @Test
+  public void suffixesOnDifferentServersAreNotEqual()
+  {
+    SuffixDescriptor suffix1 = suffix(SUFFIX_DN, server("server1.example.com"));
+    SuffixDescriptor suffix2 = suffix(SUFFIX_DN, server("server2.example.com"));
+
+    assertThat(suffix1).isNotEqualTo(suffix2);
+  }
+
+  @Test
+  public void suffixIsNotEqualToItsId()
+  {
+    SuffixDescriptor suffix = suffix(SUFFIX_DN, server("server1.example.com"));
+
+    assertThat(suffix.equals(suffix.getId())).isFalse();
+    assertThat(suffix.equals(null)).isFalse();
+  }
+
+  /**
+   * The id must not depend on the iteration order of the replica set: {@code getReplicas()}
+   * hands out a {@code HashSet} of {@code ReplicaDescriptor}s, which do not override
+   * {@code hashCode()}.
+   */
+  @Test
+  public void idIsIndependentOfTheReplicaOrder()
+  {
+    ServerDescriptor server1 = server("server1.example.com");
+    ServerDescriptor server2 = server("server2.example.com");
+    assertThat(server1.getId()).isLessThan(server2.getId());
+
+    String expectedId = SUFFIX_DN + "-" + server1.getId() + "-" + server2.getId();
+
+    assertThat(suffix(SUFFIX_DN, server1, server2).getId()).isEqualTo(expectedId);
+    assertThat(suffix(SUFFIX_DN, server2, server1).getId()).isEqualTo(expectedId);
+  }
+
+  private ServerDescriptor server(String hostName)
+  {
+    Map<ADSContext.ServerProperty, Object> adsProperties = new HashMap<>();
+    adsProperties.put(ADSContext.ServerProperty.HOST_NAME, hostName);
+    adsProperties.put(ADSContext.ServerProperty.LDAP_PORT, 1389);
+    return ServerDescriptor.createStandalone(adsProperties);
+  }
+
+  private SuffixDescriptor suffix(DN suffixDN, ServerDescriptor... servers)
+  {
+    SuffixDescriptor suffix = new SuffixDescriptor(suffixDN, replica(servers[0]));
+    for (int i = 1; i < servers.length; i++)
+    {
+      suffix.addReplica(replica(servers[i]));
+    }
+    return suffix;
+  }
+
+  private ReplicaDescriptor replica(ServerDescriptor server)
+  {
+    ReplicaDescriptor replica = new ReplicaDescriptor();
+    replica.setServer(server);
+    return replica;
+  }
+}
diff --git a/opendj-server-legacy/src/test/java/org/opends/guitools/controlpanel/datamodel/ScheduleTypeTest.java b/opendj-server-legacy/src/test/java/org/opends/guitools/controlpanel/datamodel/ScheduleTypeTest.java
new file mode 100644
index 0000000..b1f8dc4
--- /dev/null
+++ b/opendj-server-legacy/src/test/java/org/opends/guitools/controlpanel/datamodel/ScheduleTypeTest.java
@@ -0,0 +1,77 @@
+/*
+ * 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.guitools.controlpanel.datamodel;
+
+import static org.assertj.core.api.Assertions.*;
+
+import java.util.Date;
+
+import org.opends.server.DirectoryServerTestCase;
+import org.testng.annotations.Test;
+
+/** Tests the {@code equals()} contract of {@link ScheduleType}. */
+@Test(groups = { "precommit", "controlpanel" }, sequential = true)
+@SuppressWarnings("javadoc")
+public class ScheduleTypeTest extends DirectoryServerTestCase
+{
+  @Test
+  public void schedulesOfTheSameTypeAreEqual()
+  {
+    ScheduleType schedule1 = ScheduleType.createCron("0 0 * * *");
+    ScheduleType schedule2 = ScheduleType.createCron("0 0 * * *");
+
+    assertThat(schedule1).isEqualTo(schedule2);
+    assertThat(schedule1.hashCode()).isEqualTo(schedule2.hashCode());
+  }
+
+  @Test
+  public void schedulesOfDifferentTypesAreNotEqual()
+  {
+    ScheduleType launchNow = ScheduleType.createLaunchNow();
+    ScheduleType launchLater = ScheduleType.createLaunchLater(new Date(0));
+
+    assertThat(launchNow).isNotEqualTo(launchLater);
+    assertThat(launchNow).isNotEqualTo(ScheduleType.createCron("0 0 * * *"));
+  }
+
+  /**
+   * A schedule must not compare equal to a plain object sharing its textual form: such a
+   * comparison could never be symmetric.
+   */
+  @Test
+  public void scheduleIsNotEqualToAStringWithTheSameTextualForm()
+  {
+    ScheduleType schedule = ScheduleType.createLaunchNow();
+    String sameText = schedule.toString();
+
+    assertThat(schedule.equals(sameText)).isFalse();
+    assertThat(sameText.equals(schedule)).isFalse();
+  }
+
+  @Test
+  public void scheduleIsNotEqualToNull()
+  {
+    assertThat(ScheduleType.createLaunchNow().equals(null)).isFalse();
+  }
+
+  @Test
+  public void scheduleIsEqualToItself()
+  {
+    ScheduleType schedule = ScheduleType.createLaunchNow();
+
+    assertThat(schedule).isEqualTo(schedule);
+  }
+}

--
Gitblit v1.10.0