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/loggers/DebugLogPublisher.java | 37 +++++++++++++++++++++++++++----------
1 files changed, 27 insertions(+), 10 deletions(-)
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);
--
Gitblit v1.10.0