svn commit: r1303633 - in /hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common: CHANGES.txt src/main/java/org/apache/hadoop/conf/Configuration.java

[to...@apache.org]

Author: todd
Date: Thu Mar 22 00:48:56 2012
New Revision: 1303633

URL: http://svn.apache.org/viewvc?rev=1303633&view=rev
Log:
HADOOP-8157. Fix race condition in Configuration that could cause spurious ClassNotFoundExceptions after a GC. Contributed by Todd Lipcon.

Modified:
    hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt
    hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java

Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt?rev=1303633&r1=1303632&r2=1303633&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt (original)
+++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/CHANGES.txt Thu Mar 22 00:48:56 2012
@@ -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)

Modified: hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java?rev=1303633&r1=1303632&r2=1303633&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java (original)
+++ hadoop/common/branches/branch-0.23/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java Thu Mar 22 00:48:56 2012
@@ -190,6 +190,12 @@ public class Configuration implements It
     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 It
       }
     }
 
-    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 It
     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 {}
 }