Update to the latest log4j and spotbugs versions

core/src/main/spotbugs/exclude-filter.xml

@@ -43,4 +43,14 @@
     <!-- More convenient to ignore these everywhere, because it's very common and unimportant -->
     <Bug pattern="JUA_DONT_ASSERT_INSTANCEOF_IN_TESTS" />
   </Match>
+  <Match>
+    <!-- More convenient to ignore these everywhere for now. -->
+    <Bug pattern="USO_UNSAFE_STATIC_METHOD_SYNCHRONIZATION,USO_UNSAFE_METHOD_SYNCHRONIZATION,USO_UNSAFE_OBJECT_SYNCHRONIZATION,USO_UNSAFE_ACCESSIBLE_OBJECT_SYNCHRONIZATION" />
+  </Match>
+  <Match>
+    <!-- Match against any Test class -->
+    <Class name="~.*\.*Test" />
+    <!-- Test code will pass null values to ensure NullPointerExceptions are thrown -->
+    <Bug pattern="NP_NULL_PARAM_DEREF_NONVIRTUAL,NP_NULL_PARAM_DEREF_ALL_TARGETS_DANGEROUS" />
+  </Match>
 </FindBugsFilter>

core/src/test/java/org/apache/accumulo/core/logging/EscalatingLoggerTest.java

@@ -48,8 +48,8 @@ public void test() throws InterruptedException {
     // Programatically modify the Log4j2 Logging configuration to add an appender
     LoggerContext ctx = LoggerContext.getContext(false);
     Configuration cfg = ctx.getConfiguration();
-    PatternLayout layout = PatternLayout.newBuilder().withConfiguration(cfg)
-        .withPattern(PatternLayout.SIMPLE_CONVERSION_PATTERN).build();
+    PatternLayout layout = PatternLayout.newBuilder().setConfiguration(cfg)
+        .setPattern(PatternLayout.SIMPLE_CONVERSION_PATTERN).build();
     Appender appender = WriterAppender.createAppender(layout, null, writer,
         "EscalatingLoggerTestAppender", false, true);
     appender.start();

pom.xml

@@ -171,7 +171,7 @@ under the License.
     <version.curator>5.8.0</version.curator>
     <version.errorprone>2.41.0</version.errorprone>
     <version.hadoop>3.3.6</version.hadoop>
-    <version.log4j>2.25.4</version.log4j>
+    <version.log4j>2.26.1</version.log4j>
     <version.opentelemetry>1.48.0</version.opentelemetry>
     <version.powermock>2.0.9</version.powermock>
     <version.slf4j>2.0.17</version.slf4j>
@@ -277,7 +277,7 @@ under the License.
       <dependency>
         <groupId>com.github.spotbugs</groupId>
         <artifactId>spotbugs-annotations</artifactId>
-        <version>4.9.3</version>
+        <version>4.10.3</version>
       </dependency>
       <dependency>
         <groupId>com.google.auto.service</groupId>
@@ -780,7 +780,7 @@ under the License.
         <plugin>
           <groupId>com.github.spotbugs</groupId>
           <artifactId>spotbugs-maven-plugin</artifactId>
-          <version>4.9.8.2</version>
+          <version>4.10.3.0</version>
           <configuration>
             <xmlOutput>true</xmlOutput>
             <effort>Max</effort>

server/base/src/main/java/org/apache/accumulo/server/client/BulkImporter.java

@@ -475,7 +475,7 @@ private Map<Path,List<KeyExtent>> assignMapFiles(VolumeManager fs,
   }
 
   private class AssignmentTask implements Runnable {
-    final Map<Path,List<KeyExtent>> assignmentFailures;
+    private final Map<Path,List<KeyExtent>> assignmentFailures;
     HostAndPort location;
     private Map<KeyExtent,List<PathSize>> assignmentsPerTablet;
 

server/base/src/main/java/org/apache/accumulo/server/compaction/CompactionWatcher.java

@@ -45,6 +45,7 @@ public static void setTimer(LongTaskTimer ltt) {
   private final AccumuloConfiguration config;
   private static boolean watching = false;
   private static LongTaskTimer timer = null;
+  private static final Object lock = new Object();
 
   private static class ObservedCompactionInfo {
     final CompactionInfo compactionInfo;
@@ -121,11 +122,14 @@ public void run() {
     }
   }
 
-  public static synchronized void startWatching(ServerContext context) {
-    if (!watching) {
-      ThreadPools.watchCriticalScheduledTask(context.getScheduledExecutor().scheduleWithFixedDelay(
-          new CompactionWatcher(context.getConfiguration()), 10000, 10000, TimeUnit.MILLISECONDS));
-      watching = true;
+  public static void startWatching(ServerContext context) {
+    synchronized (lock) {
+      if (!watching) {
+        ThreadPools.watchCriticalScheduledTask(context.getScheduledExecutor()
+            .scheduleWithFixedDelay(new CompactionWatcher(context.getConfiguration()), 10000, 10000,
+                TimeUnit.MILLISECONDS));
+        watching = true;
+      }
     }
   }
 

server/base/src/main/java/org/apache/accumulo/server/fs/FileManager.java

@@ -111,6 +111,7 @@ public int hashCode() {
   private final long slowFilePermitMillis;
 
   private final ServerContext context;
+  private final Object lock = new Object();
 
   private class IdleFileCloser implements Runnable {
 
@@ -123,7 +124,7 @@ public void run() {
 
       // determine which files to close in a sync block, and then close the
       // files outside of the sync block
-      synchronized (FileManager.this) {
+      synchronized (lock) {
         Iterator<Entry<String,List<OpenReader>>> iter = openFiles.entrySet().iterator();
         while (iter.hasNext()) {
           Entry<String,List<OpenReader>> entry = iter.next();

server/base/src/main/java/org/apache/accumulo/server/problems/ProblemReports.java

@@ -59,6 +59,7 @@ public class ProblemReports implements Iterable<ProblemReport> {
   private static final Logger log = LoggerFactory.getLogger(ProblemReports.class);
 
   private final LRUMap<ProblemReport,Long> problemReports = new LRUMap<>(1000);
+  private static final Object lock = new Object();
 
   /**
    * use a thread pool so that reporting a problem never blocks
@@ -283,12 +284,14 @@ public Iterator<ProblemReport> iterator() {
     return iterator(null);
   }
 
-  public static synchronized ProblemReports getInstance(ServerContext context) {
-    if (instance == null) {
-      instance = new ProblemReports(context);
-    }
+  public static ProblemReports getInstance(ServerContext context) {
+    synchronized (lock) {
+      if (instance == null) {
+        instance = new ProblemReports(context);
+      }
 
-    return instance;
+      return instance;
+    }
   }
 
   public static void main(String[] args) {

server/base/src/main/java/org/apache/accumulo/server/replication/StatusUtil.java

@@ -35,6 +35,7 @@ public class StatusUtil {
   private static final Value INF_END_REPLICATION_STATUS_VALUE, CLOSED_STATUS_VALUE;
 
   private static final Status.Builder CREATED_STATUS_BUILDER;
+  private static final Object lock = new Object();
 
   static {
     CREATED_STATUS_BUILDER = Status.newBuilder();
@@ -119,11 +120,13 @@ public static Status replicatedAndIngested(Status.Builder builder, long recordsR
   /**
    * @return A {@link Status} for a new file that was just created
    */
-  public static synchronized Status fileCreated(long timeCreated) {
-    // We're using a shared builder, so we need to synchronize access on it until we make a Status
-    // (which is then immutable)
-    CREATED_STATUS_BUILDER.setCreatedTime(timeCreated);
-    return CREATED_STATUS_BUILDER.build();
+  public static Status fileCreated(long timeCreated) {
+    synchronized (lock) {
+      // We're using a shared builder, so we need to synchronize access on it until we make a Status
+      // (which is then immutable)
+      CREATED_STATUS_BUILDER.setCreatedTime(timeCreated);
+      return CREATED_STATUS_BUILDER.build();
+    }
   }
 
   /**

server/base/src/main/java/org/apache/accumulo/server/security/handler/KerberosAuthenticator.java

@@ -56,6 +56,7 @@ public class KerberosAuthenticator implements Authenticator {
   private ServerContext context;
   private String zkUserPath;
   private UserImpersonation impersonation;
+  private final Object zooCacheLock = new Object();
 
   @Override
   public void initialize(ServerContext context) {
@@ -72,7 +73,7 @@ public boolean validSecurityHandlers() {
   }
 
   private void createUserNodeInZk(String principal) throws KeeperException, InterruptedException {
-    synchronized (zooCache) {
+    synchronized (zooCacheLock) {
       zooCache.clear();
       ZooReaderWriter zoo = context.getZooReaderWriter();
       zoo.putPrivatePersistentData(zkUserPath + "/" + principal, new byte[0],
@@ -85,7 +86,7 @@ public void initializeSecurity(String principal, byte[] token) {
     try {
       // remove old settings from zookeeper first, if any
       ZooReaderWriter zoo = context.getZooReaderWriter();
-      synchronized (zooCache) {
+      synchronized (zooCacheLock) {
         zooCache.clear();
         if (zoo.exists(zkUserPath)) {
           zoo.recursiveDelete(zkUserPath, NodeMissingPolicy.SKIP);

server/base/src/main/java/org/apache/accumulo/server/security/handler/ZKAuthenticator.java

@@ -48,6 +48,7 @@ public final class ZKAuthenticator implements Authenticator {
   private ServerContext context;
   private String ZKUserPath;
   private ZooCache zooCache;
+  private final Object zooCacheLock = new Object();
 
   @Override
   public void initialize(ServerContext context) {
@@ -88,7 +89,7 @@ public void initializeSecurity(String principal, byte[] token) {
     try {
       // remove old settings from zookeeper first, if any
       ZooReaderWriter zoo = context.getZooReaderWriter();
-      synchronized (zooCache) {
+      synchronized (zooCacheLock) {
         zooCache.clear();
         if (zoo.exists(ZKUserPath)) {
           zoo.recursiveDelete(ZKUserPath, NodeMissingPolicy.SKIP);
@@ -115,7 +116,7 @@ public void initializeSecurity(String principal, byte[] token) {
    */
   private void constructUser(String user, byte[] pass)
       throws KeeperException, InterruptedException {
-    synchronized (zooCache) {
+    synchronized (zooCacheLock) {
       zooCache.clear();
       ZooReaderWriter zoo = context.getZooReaderWriter();
       zoo.putPrivatePersistentData(ZKUserPath + "/" + user, pass, NodeExistsPolicy.FAIL);
@@ -154,7 +155,7 @@ public void createUser(String principal, AuthenticationToken token)
   @Override
   public void dropUser(String user) throws AccumuloSecurityException {
     try {
-      synchronized (zooCache) {
+      synchronized (zooCacheLock) {
         zooCache.clear();
         context.getZooReaderWriter().recursiveDelete(ZKUserPath + "/" + user,
             NodeMissingPolicy.FAIL);
@@ -181,7 +182,7 @@ public void changePassword(String principal, AuthenticationToken token)
     PasswordToken pt = (PasswordToken) token;
     if (userExists(principal)) {
       try {
-        synchronized (zooCache) {
+        synchronized (zooCacheLock) {
           zooCache.clear(ZKUserPath + "/" + principal);
           context.getZooReaderWriter().putPrivatePersistentData(ZKUserPath + "/" + principal,
               ZKSecurityTool.createPass(pt.getPassword()), NodeExistsPolicy.OVERWRITE);

server/base/src/main/java/org/apache/accumulo/server/security/handler/ZKAuthorizor.java

@@ -45,6 +45,7 @@ public class ZKAuthorizor implements Authorizor {
   private ServerContext context;
   private String ZKUserPath;
   private ZooCache zooCache;
+  private final Object zooCacheLock = new Object();
 
   @Override
   public void initialize(ServerContext context) {
@@ -109,7 +110,7 @@ public void initUser(String user) throws AccumuloSecurityException {
   @Override
   public void dropUser(String user) throws AccumuloSecurityException {
     try {
-      synchronized (zooCache) {
+      synchronized (zooCacheLock) {
         ZooReaderWriter zoo = context.getZooReaderWriter();
         zoo.recursiveDelete(ZKUserPath + "/" + user + ZKUserAuths, NodeMissingPolicy.SKIP);
         zooCache.clear(ZKUserPath + "/" + user);
@@ -132,7 +133,7 @@ public void dropUser(String user) throws AccumuloSecurityException {
   public void changeAuthorizations(String user, Authorizations authorizations)
       throws AccumuloSecurityException {
     try {
-      synchronized (zooCache) {
+      synchronized (zooCacheLock) {
         zooCache.clear();
         context.getZooReaderWriter().putPersistentData(ZKUserPath + "/" + user + ZKUserAuths,
             ZKSecurityTool.convertAuthorizations(authorizations), NodeExistsPolicy.OVERWRITE);

server/base/src/main/java/org/apache/accumulo/server/security/handler/ZKPermHandler.java

@@ -59,6 +59,7 @@ public class ZKPermHandler implements PermissionHandler {
   private String ZKTablePath;
   private String ZKNamespacePath;
   private ZooCache zooCache;
+  private final Object zooCacheLock = new Object();
   private final String ZKUserSysPerms = "/System";
   private final String ZKUserTablePerms = "/Tables";
   private final String ZKUserNamespacePerms = "/Namespaces";
@@ -191,7 +192,7 @@ public void grantSystemPermission(String user, SystemPermission permission)
       }
 
       if (perms.add(permission)) {
-        synchronized (zooCache) {
+        synchronized (zooCacheLock) {
           zooCache.clear();
           zoo.putPersistentData(ZKUserPath + "/" + user + ZKUserSysPerms,
               ZKSecurityTool.convertSystemPermissions(perms), NodeExistsPolicy.OVERWRITE);
@@ -220,7 +221,7 @@ public void grantTablePermission(String user, String table, TablePermission perm
 
     try {
       if (tablePerms.add(permission)) {
-        synchronized (zooCache) {
+        synchronized (zooCacheLock) {
           zooCache.clear(ZKUserPath + "/" + user + ZKUserTablePerms + "/" + table);
           zoo.putPersistentData(ZKUserPath + "/" + user + ZKUserTablePerms + "/" + table,
               ZKSecurityTool.convertTablePermissions(tablePerms), NodeExistsPolicy.OVERWRITE);
@@ -250,7 +251,7 @@ public void grantNamespacePermission(String user, String namespace,
 
     try {
       if (namespacePerms.add(permission)) {
-        synchronized (zooCache) {
+        synchronized (zooCacheLock) {
           zooCache.clear(ZKUserPath + "/" + user + ZKUserNamespacePerms + "/" + namespace);
           zoo.putPersistentData(ZKUserPath + "/" + user + ZKUserNamespacePerms + "/" + namespace,
               ZKSecurityTool.convertNamespacePermissions(namespacePerms),
@@ -281,7 +282,7 @@ public void revokeSystemPermission(String user, SystemPermission permission)
 
     try {
       if (sysPerms.remove(permission)) {
-        synchronized (zooCache) {
+        synchronized (zooCacheLock) {
           zooCache.clear();
           zoo.putPersistentData(ZKUserPath + "/" + user + ZKUserSysPerms,
               ZKSecurityTool.convertSystemPermissions(sysPerms), NodeExistsPolicy.OVERWRITE);
@@ -367,7 +368,7 @@ public void revokeNamespacePermission(String user, String namespace,
   @Override
   public void cleanTablePermissions(String table) throws AccumuloSecurityException {
     try {
-      synchronized (zooCache) {
+      synchronized (zooCacheLock) {
         zooCache.clear();
         for (String user : zooCache.getChildren(ZKUserPath)) {
           zoo.recursiveDelete(ZKUserPath + "/" + user + ZKUserTablePerms + "/" + table,
@@ -387,7 +388,7 @@ public void cleanTablePermissions(String table) throws AccumuloSecurityException
   @Override
   public void cleanNamespacePermissions(String namespace) throws AccumuloSecurityException {
     try {
-      synchronized (zooCache) {
+      synchronized (zooCacheLock) {
         zooCache.clear();
         for (String user : zooCache.getChildren(ZKUserPath)) {
           zoo.recursiveDelete(ZKUserPath + "/" + user + ZKUserNamespacePerms + "/" + namespace,
@@ -471,7 +472,7 @@ public void initUser(String user) throws AccumuloSecurityException {
    */
   private void createTablePerm(String user, TableId table, Set<TablePermission> perms)
       throws KeeperException, InterruptedException {
-    synchronized (zooCache) {
+    synchronized (zooCacheLock) {
       zooCache.clear();
       zoo.putPersistentData(ZKUserPath + "/" + user + ZKUserTablePerms + "/" + table,
           ZKSecurityTool.convertTablePermissions(perms), NodeExistsPolicy.FAIL);
@@ -484,7 +485,7 @@ private void createTablePerm(String user, TableId table, Set<TablePermission> pe
    */
   private void createNamespacePerm(String user, NamespaceId namespace,
       Set<NamespacePermission> perms) throws KeeperException, InterruptedException {
-    synchronized (zooCache) {
+    synchronized (zooCacheLock) {
       zooCache.clear();
       zoo.putPersistentData(ZKUserPath + "/" + user + ZKUserNamespacePerms + "/" + namespace,
           ZKSecurityTool.convertNamespacePermissions(perms), NodeExistsPolicy.FAIL);
@@ -494,7 +495,7 @@ private void createNamespacePerm(String user, NamespaceId namespace,
   @Override
   public void cleanUser(String user) throws AccumuloSecurityException {
     try {
-      synchronized (zooCache) {
+      synchronized (zooCacheLock) {
         zoo.recursiveDelete(ZKUserPath + "/" + user + ZKUserSysPerms, NodeMissingPolicy.SKIP);
         zoo.recursiveDelete(ZKUserPath + "/" + user + ZKUserTablePerms, NodeMissingPolicy.SKIP);
         zoo.recursiveDelete(ZKUserPath + "/" + user + ZKUserNamespacePerms, NodeMissingPolicy.SKIP);