HADOOP-8157. Fix race condition in Configuration that could cause spurious ClassNotFoundExceptions after a GC. Contributed by Todd Lipcon.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-0.23@1303633 13f79535-47bb-0310-9956-ffa450edef68

hadoop-common-project/hadoop-common/CHANGES.txt

@@ -149,6 +149,9 @@ Release 0.23.3 - UNRELEASED
 
     HADOOP-8191. SshFenceByTcpPort uses netcat incorrectly (todd)
 
+    HADOOP-8157. Fix race condition in Configuration that could cause spurious
+    ClassNotFoundExceptions after a GC. (todd)
+
   BREAKDOWN OF HADOOP-7454 SUBTASKS
 
     HADOOP-7455. HA: Introduce HA Service Protocol Interface. (suresh)

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java

@@ -189,6 +189,12 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
   private static final Map<ClassLoader, Map<String, Class<?>>>
     CACHE_CLASSES = new WeakHashMap<ClassLoader, Map<String, Class<?>>>();
 
+  /**
+   * Sentinel value to store negative cache results in {@link #CACHE_CLASSES}.
+   */
+  private static final Class<?> NEGATIVE_CACHE_SENTINEL =
+    NegativeCacheSentinel.class;
+
   /**
    * Stores the mapping of key to the resource which modifies or loads 
    * the key most recently
@@ -1194,24 +1200,24 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
       }
     }
 
-    Class<?> clazz = null;
-    if (!map.containsKey(name)) {
+    Class<?> clazz = map.get(name);
+    if (clazz == null) {
       try {
         clazz = Class.forName(name, true, classLoader);
       } catch (ClassNotFoundException e) {
-        map.put(name, null); //cache negative that class is not found
+        // Leave a marker that the class isn't found
+        map.put(name, NEGATIVE_CACHE_SENTINEL);
         return null;
       }
       // two putters can race here, but they'll put the same class
       map.put(name, clazz);
-    } else { // check already performed on this class name
-      clazz = map.get(name);
-      if (clazz == null) { // found the negative
-        return null;
-      }
+      return clazz;
+    } else if (clazz == NEGATIVE_CACHE_SENTINEL) {
+      return null; // not found
+    } else {
+      // cache hit
+      return clazz;
     }
-
-    return clazz;
   }
 
   /** 
@@ -1913,4 +1919,10 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
     Configuration.addDeprecation("fs.default.name", 
                new String[]{CommonConfigurationKeys.FS_DEFAULT_NAME_KEY});
   }
+  
+  /**
+   * A unique class which is used as a sentinel value in the caching
+   * for getClassByName. {@see Configuration#getClassByNameOrNull(String)}
+   */
+  private static abstract class NegativeCacheSentinel {}
 }