Update to the latest log4j and spotbugs versions

core/src/main/java/org/apache/accumulo/core/classloader/ClassLoaderUtil.java

@@ -32,6 +32,7 @@ public class ClassLoaderUtil {
 
   private static final Logger LOG = LoggerFactory.getLogger(ClassLoaderUtil.class);
   private static ContextClassLoaderFactory FACTORY;
+  private static final Object FACTORY_LOCK = new Object();
 
   private ClassLoaderUtil() {
     // cannot construct; static utilities only
@@ -40,29 +41,33 @@ private ClassLoaderUtil() {
   /**
    * Initialize the ContextClassLoaderFactory
    */
-  public static synchronized void initContextFactory(AccumuloConfiguration conf) {
-    if (FACTORY == null) {
-      LOG.debug("Creating {}", ContextClassLoaderFactory.class.getName());
-      String factoryName = conf.get(Property.GENERAL_CONTEXT_CLASSLOADER_FACTORY);
-      if (factoryName == null || factoryName.isEmpty()) {
-        // load the default implementation
-        LOG.info("Using default {}, which is subject to change in a future release",
-            ContextClassLoaderFactory.class.getName());
-        FACTORY = new DefaultContextClassLoaderFactory(conf);
-      } else {
-        // load user's selected implementation and provide it with the service environment
-        try {
-          var factoryClass = Class.forName(factoryName).asSubclass(ContextClassLoaderFactory.class);
-          LOG.info("Creating {}: {}", ContextClassLoaderFactory.class.getName(), factoryName);
-          FACTORY = factoryClass.getDeclaredConstructor().newInstance();
-          FACTORY.init(() -> new ConfigurationImpl(conf));
-        } catch (ReflectiveOperationException e) {
-          throw new IllegalStateException("Unable to load and initialize class: " + factoryName, e);
+  public static void initContextFactory(AccumuloConfiguration conf) {
+    synchronized (FACTORY_LOCK) {
+      if (FACTORY == null) {
+        LOG.debug("Creating {}", ContextClassLoaderFactory.class.getName());
+        String factoryName = conf.get(Property.GENERAL_CONTEXT_CLASSLOADER_FACTORY);
+        if (factoryName == null || factoryName.isEmpty()) {
+          // load the default implementation
+          LOG.info("Using default {}, which is subject to change in a future release",
+              ContextClassLoaderFactory.class.getName());
+          FACTORY = new DefaultContextClassLoaderFactory(conf);
+        } else {
+          // load user's selected implementation and provide it with the service environment
+          try {
+            var factoryClass =
+                Class.forName(factoryName).asSubclass(ContextClassLoaderFactory.class);
+            LOG.info("Creating {}: {}", ContextClassLoaderFactory.class.getName(), factoryName);
+            FACTORY = factoryClass.getDeclaredConstructor().newInstance();
+            FACTORY.init(() -> new ConfigurationImpl(conf));
+          } catch (ReflectiveOperationException e) {
+            throw new IllegalStateException("Unable to load and initialize class: " + factoryName,
+                e);
+          }
         }
+      } else {
+        LOG.debug("{} already initialized with {}.", ContextClassLoaderFactory.class.getName(),
+            FACTORY.getClass().getName());
       }
-    } else {
-      LOG.debug("{} already initialized with {}.", ContextClassLoaderFactory.class.getName(),
-          FACTORY.getClass().getName());
     }
   }
 
@@ -72,8 +77,10 @@ static ContextClassLoaderFactory getContextFactory() {
   }
 
   // for testing
-  public static synchronized void resetContextFactoryForTests() {
-    FACTORY = null;
+  public static void resetContextFactoryForTests() {
+    synchronized (FACTORY_LOCK) {
+      FACTORY = null;
+    }
   }
 
   @SuppressWarnings("deprecation")

core/src/main/java/org/apache/accumulo/core/clientImpl/ScannerImpl.java

@@ -82,6 +82,7 @@ public boolean removeEldestEntry(Map.Entry<ScannerIterator,Long> eldest) {
       return size() > MAX_ENTRIES;
     }
   };
+  private final Object scannerLock = new Object();
 
   /**
    * This is used for ScannerIterators to report their activity back to the scanner that created
@@ -90,15 +91,15 @@ public boolean removeEldestEntry(Map.Entry<ScannerIterator,Long> eldest) {
   class Reporter {
 
     void readBatch(ScannerIterator iter) {
-      synchronized (ScannerImpl.this) {
+      synchronized (scannerLock) {
         // This iter just had some activity, so access it in map so it becomes the most recently
         // used.
         iters.get(iter);
       }
     }
 
     void finished(ScannerIterator iter) {
-      synchronized (ScannerImpl.this) {
+      synchronized (scannerLock) {
         iters.remove(iter);
       }
     }

core/src/main/java/org/apache/accumulo/core/clientImpl/TabletLocator.java

@@ -60,6 +60,8 @@ boolean isValid() {
     return isValid;
   }
 
+  private static final Object lock = new Object();
+
   public abstract TabletLocation locateTablet(ClientContext context, Text row, boolean skipRow,
       boolean retry) throws AccumuloException, AccumuloSecurityException, TableNotFoundException;
 
@@ -118,12 +120,14 @@ public boolean equals(LocatorKey lk) {
       new HashMap<>();
   private static boolean enabled = true;
 
-  public static synchronized void clearLocators() {
-    for (TabletLocator locator : locators.values()) {
-      locator.isValid = false;
+  public static void clearLocators() {
+    synchronized (lock) {
+      for (TabletLocator locator : locators.values()) {
+        locator.isValid = false;
+      }
+      locators.clear();
+      offlineLocators.clear();
     }
-    locators.clear();
-    offlineLocators.clear();
   }
 
   static synchronized boolean isEnabled() {
@@ -139,36 +143,39 @@ static synchronized void enable() {
     enabled = true;
   }
 
-  public static synchronized TabletLocator getLocator(ClientContext context, TableId tableId) {
-    Preconditions.checkState(enabled, "The Accumulo singleton that that tracks tablet locations is "
-        + "disabled. This is likely caused by all AccumuloClients being closed or garbage collected");
-
-    clearUnusedTables(context);
-
-    TableState state = context.getTableState(tableId);
-    LocatorKey key = new LocatorKey(context.getInstanceID(), tableId);
-    if (state == TableState.OFFLINE) {
-      locators.remove(key);
-      return offlineLocators.computeIfAbsent(key,
-          f -> new OfflineTabletLocatorImpl(context, tableId));
-    } else {
-      offlineLocators.remove(key);
-      TabletLocator tl = locators.get(key);
-      if (tl == null) {
-        MetadataLocationObtainer mlo = new MetadataLocationObtainer();
-
-        if (RootTable.ID.equals(tableId)) {
-          tl = new RootTabletLocator(context.getTServerLockChecker());
-        } else if (MetadataTable.ID.equals(tableId)) {
-          tl = new TabletLocatorImpl(MetadataTable.ID, getLocator(context, RootTable.ID), mlo,
-              context.getTServerLockChecker());
-        } else {
-          tl = new TabletLocatorImpl(tableId, getLocator(context, MetadataTable.ID), mlo,
-              context.getTServerLockChecker());
+  public static TabletLocator getLocator(ClientContext context, TableId tableId) {
+    synchronized (lock) {
+      Preconditions.checkState(enabled,
+          "The Accumulo singleton that that tracks tablet locations is "
+              + "disabled. This is likely caused by all AccumuloClients being closed or garbage collected");
+
+      clearUnusedTables(context);
+
+      TableState state = context.getTableState(tableId);
+      LocatorKey key = new LocatorKey(context.getInstanceID(), tableId);
+      if (state == TableState.OFFLINE) {
+        locators.remove(key);
+        return offlineLocators.computeIfAbsent(key,
+            f -> new OfflineTabletLocatorImpl(context, tableId));
+      } else {
+        offlineLocators.remove(key);
+        TabletLocator tl = locators.get(key);
+        if (tl == null) {
+          MetadataLocationObtainer mlo = new MetadataLocationObtainer();
+
+          if (RootTable.ID.equals(tableId)) {
+            tl = new RootTabletLocator(context.getTServerLockChecker());
+          } else if (MetadataTable.ID.equals(tableId)) {
+            tl = new TabletLocatorImpl(MetadataTable.ID, getLocator(context, RootTable.ID), mlo,
+                context.getTServerLockChecker());
+          } else {
+            tl = new TabletLocatorImpl(tableId, getLocator(context, MetadataTable.ID), mlo,
+                context.getTServerLockChecker());
+          }
+          locators.put(key, tl);
         }
-        locators.put(key, tl);
+        return tl;
       }
-      return tl;
     }
 
   }
@@ -177,9 +184,11 @@ public static synchronized TabletLocator getLocator(ClientContext context, Table
    * Checks if a table id is present in the cache w/o creating it.
    */
   @VisibleForTesting
-  public static synchronized boolean isPresent(ClientContext context, TableId tableId) {
-    LocatorKey key = new LocatorKey(context.getInstanceID(), tableId);
-    return locators.containsKey(key) || offlineLocators.containsKey(key);
+  public static boolean isPresent(ClientContext context, TableId tableId) {
+    synchronized (lock) {
+      LocatorKey key = new LocatorKey(context.getInstanceID(), tableId);
+      return locators.containsKey(key) || offlineLocators.containsKey(key);
+    }
   }
 
   private static Duration clearFrequency = Duration.ofMinutes(10);
@@ -188,10 +197,13 @@ public static synchronized boolean isPresent(ClientContext context, TableId tabl
    * Sets how often checks for unused tables are done
    */
   @VisibleForTesting
-  public static synchronized void setClearFrequency(Duration frequency) {
-    Preconditions.checkArgument(frequency != null && !frequency.isNegative() && !frequency.isZero(),
-        "frequency:%s", frequency);
-    clearFrequency = frequency;
+  public static void setClearFrequency(Duration frequency) {
+    synchronized (lock) {
+      Preconditions.checkArgument(
+          frequency != null && !frequency.isNegative() && !frequency.isZero(), "frequency:%s",
+          frequency);
+      clearFrequency = frequency;
+    }
   }
 
   private static final Timer lastClearTimer = Timer.startNew();

core/src/main/java/org/apache/accumulo/core/clientImpl/TabletServerBatchReaderIterator.java

@@ -740,6 +740,7 @@ private static class TimeoutTracker {
     final String server;
     final Set<String> badServers;
     final long timeOut;
+    private final Object lock = new Object();
 
     // When failures happen, rpc task to scan a server may be requeued in a thread pool. These two
     // variables track failures across task running in those thread pools.
@@ -771,7 +772,7 @@ void check() throws IOException {
 
       void madeProgress() {
         activityTime = System.currentTimeMillis();
-        synchronized (TimeoutTracker.this) {
+        synchronized (lock) {
           firstErrorTime = null;
           firstAllFailureTime = null;
         }

core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ServiceLock.java

@@ -69,6 +69,8 @@ public class ServiceLock implements Watcher {
 
   private static final String ZLOCK_PREFIX = "zlock#";
 
+  private static final Object lock = new Object();
+
   private static class Prefix {
     private final String prefix;
 
@@ -370,7 +372,7 @@ public void process(WatchedEvent event) {
           if (event.getType() == EventType.NodeDeleted && event.getPath().equals(nodeToWatch)) {
             LOG.debug("[{}] Detected deletion of prior node {}, attempting to acquire lock; {}",
                 vmLockPrefix, nodeToWatch, event);
-            synchronized (ServiceLock.this) {
+            synchronized (lock) {
               try {
                 if (createdNodeName != null) {
                   determineLockOwnership(lw);
@@ -390,7 +392,7 @@ public void process(WatchedEvent event) {
 
           if (event.getState() == KeeperState.Expired
               || event.getState() == KeeperState.Disconnected) {
-            synchronized (ServiceLock.this) {
+            synchronized (lock) {
               if (lockNodeName == null) {
                 LOG.info("Zookeeper Session expired / disconnected; {}", event);
                 lw.failedToAcquireLock(
@@ -400,7 +402,7 @@ public void process(WatchedEvent event) {
             renew = false;
           }
           if (renew) {
-            synchronized (ServiceLock.this) {
+            synchronized (lock) {
               if (createdNodeName != null) {
                 try {
                   Stat restat = zooKeeper.exists(nodeToWatch, this);
@@ -524,7 +526,7 @@ private void failedToAcquireLock() {
 
         @Override
         public void process(WatchedEvent event) {
-          synchronized (ServiceLock.this) {
+          synchronized (lock) {
             if (lockNodeName != null && event.getType() == EventType.NodeDeleted
                 && event.getPath().equals(path + "/" + lockNodeName)) {
               LOG.debug("[{}] {} was deleted; {}", vmLockPrefix, lockNodeName, event);

core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/impl/BlockCacheManagerFactory.java

@@ -28,6 +28,7 @@
 public class BlockCacheManagerFactory {
 
   private static final Logger LOG = LoggerFactory.getLogger(BlockCacheManager.class);
+  private static final Object lock = new Object();
 
   /**
    * Get the BlockCacheFactory specified by the property 'tserver.cache.factory.class' using the
@@ -37,16 +38,17 @@ public class BlockCacheManagerFactory {
    * @return block cache manager instance
    * @throws Exception error loading block cache manager implementation class
    */
-  public static synchronized BlockCacheManager getInstance(AccumuloConfiguration conf)
-      throws Exception {
-    @SuppressWarnings("deprecation")
-    var cacheManagerProp =
-        conf.resolve(Property.GENERAL_CACHE_MANAGER_IMPL, Property.TSERV_CACHE_MANAGER_IMPL);
-    String impl = conf.get(cacheManagerProp);
-    Class<? extends BlockCacheManager> clazz =
-        ClassLoaderUtil.loadClass(impl, BlockCacheManager.class);
-    LOG.info("Created new block cache manager of type: {}", clazz.getSimpleName());
-    return clazz.getDeclaredConstructor().newInstance();
+  public static BlockCacheManager getInstance(AccumuloConfiguration conf) throws Exception {
+    synchronized (lock) {
+      @SuppressWarnings("deprecation")
+      var cacheManagerProp =
+          conf.resolve(Property.GENERAL_CACHE_MANAGER_IMPL, Property.TSERV_CACHE_MANAGER_IMPL);
+      String impl = conf.get(cacheManagerProp);
+      Class<? extends BlockCacheManager> clazz =
+          ClassLoaderUtil.loadClass(impl, BlockCacheManager.class);
+      LOG.info("Created new block cache manager of type: {}", clazz.getSimpleName());
+      return clazz.getDeclaredConstructor().newInstance();
+    }
   }
 
   /**
@@ -56,15 +58,16 @@ public static synchronized BlockCacheManager getInstance(AccumuloConfiguration c
    * @return block cache manager instance
    * @throws Exception error loading block cache manager implementation class
    */
-  public static synchronized BlockCacheManager getClientInstance(AccumuloConfiguration conf)
-      throws Exception {
-    @SuppressWarnings("deprecation")
-    var cacheManagerProp =
-        conf.resolve(Property.GENERAL_CACHE_MANAGER_IMPL, Property.TSERV_CACHE_MANAGER_IMPL);
-    String impl = conf.get(cacheManagerProp);
-    Class<? extends BlockCacheManager> clazz =
-        Class.forName(impl).asSubclass(BlockCacheManager.class);
-    LOG.info("Created new block cache factory of type: {}", clazz.getSimpleName());
-    return clazz.getDeclaredConstructor().newInstance();
+  public static BlockCacheManager getClientInstance(AccumuloConfiguration conf) throws Exception {
+    synchronized (lock) {
+      @SuppressWarnings("deprecation")
+      var cacheManagerProp =
+          conf.resolve(Property.GENERAL_CACHE_MANAGER_IMPL, Property.TSERV_CACHE_MANAGER_IMPL);
+      String impl = conf.get(cacheManagerProp);
+      Class<? extends BlockCacheManager> clazz =
+          Class.forName(impl).asSubclass(BlockCacheManager.class);
+      LOG.info("Created new block cache factory of type: {}", clazz.getSimpleName());
+      return clazz.getDeclaredConstructor().newInstance();
+    }
   }
 }

core/src/main/java/org/apache/accumulo/core/file/rfile/BlockIndex.java

@@ -33,6 +33,8 @@
 
 public class BlockIndex implements Weighable {
 
+  private final Object lock = new Object();
+
   private BlockIndex() {}
 
   public static BlockIndex getIndex(CachedBlockRead cacheBlock, IndexEntry indexEntry)
@@ -214,16 +216,18 @@ BlockIndexEntry[] getIndexEntries() {
   }
 
   @Override
-  public synchronized int weight() {
-    int weight = 0;
-    if (blockIndex != null) {
-      for (BlockIndexEntry blockIndexEntry : blockIndex) {
-        weight += blockIndexEntry.weight();
+  public int weight() {
+    synchronized (lock) {
+      int weight = 0;
+      if (blockIndex != null) {
+        for (BlockIndexEntry blockIndexEntry : blockIndex) {
+          weight += blockIndexEntry.weight();
+        }
       }
-    }
 
-    weight +=
-        ClassSize.ATOMIC_INTEGER + ClassSize.OBJECT + 2 * ClassSize.REFERENCE + ClassSize.ARRAY;
-    return weight;
+      weight +=
+          ClassSize.ATOMIC_INTEGER + ClassSize.OBJECT + 2 * ClassSize.REFERENCE + ClassSize.ARRAY;
+      return weight;
+    }
   }
 }