This is an automated email from the ASF dual-hosted git repository.

stevel pushed a commit to branch branch-3.3
in repository https://gitbox.apache.org/repos/asf/hadoop.git


The following commit(s) were added to refs/heads/branch-3.3 by this push:
     new 638d9dc71310 HADOOP-19119. Spotbugs: possible NPE in 
org.apache.hadoop.crypto.key.kms.ValueQueue.getSize() (#6642)
638d9dc71310 is described below

commit 638d9dc71310424760ddb6a8c94476e779626b90
Author: Steve Loughran <ste...@cloudera.com>
AuthorDate: Tue Mar 19 17:18:07 2024 +0000

    HADOOP-19119. Spotbugs: possible NPE in 
org.apache.hadoop.crypto.key.kms.ValueQueue.getSize() (#6642)
    
    Spotbugs is mistaken here as it doesn't observer the read/write locks used
    to manage exclusive access to the maps.
    
    * cache the value between checks
    * tag as @VisibleForTesting
    
    Contributed by Steve Loughran
---
 .../java/org/apache/hadoop/crypto/key/kms/ValueQueue.java    | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git 
a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/ValueQueue.java
 
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/ValueQueue.java
index 7162d77d3b8d..4c72634dae37 100644
--- 
a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/ValueQueue.java
+++ 
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/ValueQueue.java
@@ -33,7 +33,8 @@ import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.locks.ReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
-import org.apache.hadoop.thirdparty.com.google.common.base.Preconditions;
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.util.Preconditions;
 import org.apache.hadoop.thirdparty.com.google.common.cache.CacheBuilder;
 import org.apache.hadoop.thirdparty.com.google.common.cache.CacheLoader;
 import org.apache.hadoop.thirdparty.com.google.common.cache.LoadingCache;
@@ -318,8 +319,9 @@ public class ValueQueue <E> {
   /**
    * Get size of the Queue for keyName. This is only used in unit tests.
    * @param keyName the key name
-   * @return int queue size
+   * @return int queue size. Zero means the queue is empty or the key does not 
exist.
    */
+  @VisibleForTesting
   public int getSize(String keyName) {
     readLock(keyName);
     try {
@@ -327,10 +329,12 @@ public class ValueQueue <E> {
       // since that will have the side effect of populating the cache.
       Map<String, LinkedBlockingQueue<E>> map =
           keyQueues.getAllPresent(Arrays.asList(keyName));
-      if (map.get(keyName) == null) {
+      final LinkedBlockingQueue<E> linkedQueue = map.get(keyName);
+      if (linkedQueue == null) {
         return 0;
+      } else {
+        return linkedQueue.size();
       }
-      return map.get(keyName).size();
     } finally {
       readUnlock(keyName);
     }


---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-commits-h...@hadoop.apache.org

Reply via email to