You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ba...@apache.org on 2007/09/05 12:13:59 UTC

svn commit: r572930 - in /commons/proper/lang/trunk/src/java/org/apache/commons/lang: enum/Enum.java enums/Enum.java

Author: bayard
Date: Wed Sep  5 03:13:49 2007
New Revision: 572930

URL: http://svn.apache.org/viewvc?rev=572930&view=rev
Log:
Applying Jason Madden's patch from LANG-334 to provide enums.Enum with optimized thread safety. As Jason's used this in production I think it's fair to use it rather than the simpler Collections.synchronizedMap(..). I've also applied the patch to the enum.Enum

Modified:
    commons/proper/lang/trunk/src/java/org/apache/commons/lang/enum/Enum.java
    commons/proper/lang/trunk/src/java/org/apache/commons/lang/enums/Enum.java

Modified: commons/proper/lang/trunk/src/java/org/apache/commons/lang/enum/Enum.java
URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/java/org/apache/commons/lang/enum/Enum.java?rev=572930&r1=572929&r2=572930&view=diff
==============================================================================
--- commons/proper/lang/trunk/src/java/org/apache/commons/lang/enum/Enum.java (original)
+++ commons/proper/lang/trunk/src/java/org/apache/commons/lang/enum/Enum.java Wed Sep  5 03:13:49 2007
@@ -256,7 +256,11 @@
     /**
      * <code>Map</code>, key of class name, value of <code>Entry</code>.
      */
-    private static final Map cEnumClasses = new WeakHashMap();
+    private static Map cEnumClasses
+        // LANG-334: To avoid exposing a mutating map,
+        // we copy it each time we add to it. This is cheaper than
+        // using a synchronized map since we are almost entirely reads
+        = new WeakHashMap();
     
     /**
      * The string representation of the Enum.
@@ -349,12 +353,17 @@
         if (ok == false) {
             throw new IllegalArgumentException("getEnumClass() must return a superclass of this class");
         }
-        
-        // create entry
-        Entry entry = (Entry) cEnumClasses.get(enumClass);
-        if (entry == null) {
-            entry = createEntry(enumClass);
-            cEnumClasses.put(enumClass, entry);
+
+        Entry entry;
+        synchronized( Enum.class ) { // LANG-334
+            // create entry
+            entry = (Entry) cEnumClasses.get(enumClass);
+            if (entry == null) {
+                entry = createEntry(enumClass);
+                Map myMap = new WeakHashMap( cEnumClasses );
+                myMap.put(enumClass, entry);
+                cEnumClasses = myMap;
+            }
         }
         if (entry.map.containsKey(name)) {
             throw new IllegalArgumentException("The Enum name must be unique, '" + name + "' has already been added");

Modified: commons/proper/lang/trunk/src/java/org/apache/commons/lang/enums/Enum.java
URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/java/org/apache/commons/lang/enums/Enum.java?rev=572930&r1=572929&r2=572930&view=diff
==============================================================================
--- commons/proper/lang/trunk/src/java/org/apache/commons/lang/enums/Enum.java (original)
+++ commons/proper/lang/trunk/src/java/org/apache/commons/lang/enums/Enum.java Wed Sep  5 03:13:49 2007
@@ -302,7 +302,11 @@
     /**
      * <code>Map</code>, key of class name, value of <code>Entry</code>.
      */
-    private static final Map cEnumClasses = new WeakHashMap();
+    private static Map cEnumClasses
+        // LANG-334: To avoid exposing a mutating map,
+        // we copy it each time we add to it. This is cheaper than
+        // using a synchronized map since we are almost entirely reads
+        = new WeakHashMap();
     
     /**
      * The string representation of the Enum.
@@ -345,7 +349,7 @@
          * <p>Restrictive constructor.</p>
          */
         protected Entry() {
-          super();
+            super();
         }
     }
 
@@ -395,12 +399,17 @@
         if (ok == false) {
             throw new IllegalArgumentException("getEnumClass() must return a superclass of this class");
         }
-        
-        // create entry
-        Entry entry = (Entry) cEnumClasses.get(enumClass);
-        if (entry == null) {
-            entry = createEntry(enumClass);
-            cEnumClasses.put(enumClass, entry);
+
+        Entry entry;
+        synchronized( Enum.class ) { // LANG-334
+            // create entry
+            entry = (Entry) cEnumClasses.get(enumClass);
+            if (entry == null) {
+                entry = createEntry(enumClass);
+                Map myMap = new WeakHashMap( cEnumClasses );
+                myMap.put(enumClass, entry);
+                cEnumClasses = myMap;
+            }
         }
         if (entry.map.containsKey(name)) {
             throw new IllegalArgumentException("The Enum name must be unique, '" + name + "' has already been added");