Commits

Fabian Kraemer  committed b271439

JRA-26808: Prevent recursive wrapping of DirtyFlagMap's internal map into UnmodifiableMap.

  • Participants
  • Parent commits 1f2adde
  • Branches quartz_1-5-1-atlassian-x

Comments (0)

Files changed (1)

File src/java/org/quartz/utils/DirtyFlagMap.java

 import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
  * <p>
  * 
  * @author James House
  */
-public class DirtyFlagMap implements Map, Cloneable, java.io.Serializable {
+public class DirtyFlagMap<K,V> implements Map<K,V>, Cloneable, java.io.Serializable {
 
     /*
      * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     private static final long serialVersionUID = 1433884852607126222L;
 
     private boolean dirty = false;
-    private transient boolean locked = false;
-    private Map map;
+    private transient AtomicBoolean locked = new AtomicBoolean();
+    private Map<K,V> mutableMap;
+    private transient Map<K,V> exposedMap;
 
     /*
      * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      * Create a DirtyFlagMap that 'wraps' the given <code>Map</code>.
      * </p>
      */
-    public DirtyFlagMap(Map mapToWrap) {
-        if (mapToWrap == null)
+    public DirtyFlagMap(Map<K,V> mutableMap) {
+        if (mutableMap == null)
                 throw new IllegalArgumentException("mapToWrap cannot be null!");
 
-        map = mapToWrap;
+        this.mutableMap = mutableMap;
+        exposedMap = mutableMap;
     }
 
     /**
      * @see java.util.HashMap
      */
     public DirtyFlagMap() {
-        map = new HashMap();
+        this(new HashMap<K,V>());
     }
 
     /**
      * @see java.util.HashMap
      */
     public DirtyFlagMap(int initialCapacity) {
-        map = new HashMap(initialCapacity);
+        this(new HashMap<K,V>(initialCapacity));
     }
 
     /**
      * @see java.util.HashMap
      */
     public DirtyFlagMap(int initialCapacity, float loadFactor) {
-        map = new HashMap(initialCapacity, loadFactor);
+        this(new HashMap<K,V>(initialCapacity, loadFactor));
     }
 
     /*
 
 
     public void setMutable(boolean mutable) {
-        this.locked = !mutable;
-        if(locked)
-            map = Collections.unmodifiableMap(map);
-        else
-            map = new HashMap(map);
+        if(locked.compareAndSet(!mutable, mutable))
+        {
+            // the reference is leaked anyway, e.g. getWrappedMap and entrySet, so no further attempt for
+            // synchronisation. normally you'd have a custom decorator blocking modification if the flag is set, but
+            // since I don't know if callers already rely on or check for UnmodifiableMap I'll leave it this way
+            if(mutable)
+            {
+                exposedMap = mutableMap;
+            }
+            else
+            {
+                exposedMap = Collections.unmodifiableMap(mutableMap);
+            }
+        }
     }
     
     
     public boolean isMutable() {
-        return !locked;
+        return !locked.get();
     }
     
     /**
      * Get a direct handle to the underlying Map.
      * </p>
      */
-    public Map getWrappedMap() {
-        return map;
+    public Map<K,V> getWrappedMap() {
+        return exposedMap;
     }
 
+    @Override
     public void clear() {
         dirty = true;
 
-        map.clear();
+        exposedMap.clear();
     }
 
+    @Override
     public boolean containsKey(Object key) {
-        return map.containsKey(key);
+        return exposedMap.containsKey(key);
     }
 
+    @Override
     public boolean containsValue(Object val) {
-        return map.containsValue(val);
+        return exposedMap.containsValue(val);
     }
 
-    public Set entrySet() {
-        return map.entrySet();
+    @Override
+    public Set<Map.Entry<K, V>> entrySet() {
+        return exposedMap.entrySet();
     }
 
+    @SuppressWarnings("unchecked")
+    @Override
     public boolean equals(Object obj) {
         if (obj == null || !(obj instanceof DirtyFlagMap)) return false;
 
-        return map.equals(((DirtyFlagMap) obj).getWrappedMap());
+        return exposedMap.equals(((DirtyFlagMap<K,V>) obj).getWrappedMap());
     }
 
-    public Object get(Object key) {
-        return map.get(key);
+    @Override
+    public V get(Object key) {
+        return exposedMap.get(key);
     }
 
+    @Override
     public boolean isEmpty() {
-        return map.isEmpty();
+        return exposedMap.isEmpty();
     }
 
-    public Set keySet() {
-        return map.keySet();
+    @Override
+    public Set<K> keySet() {
+        return exposedMap.keySet();
     }
 
-    public Object put(Object key, Object val) {
+    @Override
+    public V put(K key, V val) {
         dirty = true;
 
-        return map.put(key, val);
+        return exposedMap.put(key, val);
     }
 
-    public void putAll(Map t) {
+    @Override
+    public void putAll(Map<? extends K, ? extends V> t) {
         if (!t.isEmpty()) dirty = true;
 
-        map.putAll(t);
+        exposedMap.putAll(t);
     }
 
-    public Object remove(Object key) {
-        Object obj = map.remove(key);
+    @Override
+    public V remove(Object key) {
+        V obj = exposedMap.remove(key);
 
         if (obj != null) dirty = true;
 
         return obj;
     }
 
+    @Override
     public int size() {
-        return map.size();
+        return exposedMap.size();
     }
 
-    public Collection values() {
-        return map.values();
+    @Override
+    public Collection<V>  values() {
+        return exposedMap.values();
     }
 
+    // FIXME is this used and if so why only clone the internal map if it's mutable?
+    @Override
+    @SuppressWarnings({ "unchecked" })
     public Object clone() {
         DirtyFlagMap copy;
         try {
             copy = (DirtyFlagMap) super.clone();
-            if (map instanceof HashMap)
-                    copy.map = (Map) ((HashMap) map).clone();
+            if (exposedMap instanceof HashMap)
+                    copy.exposedMap = (Map) ((HashMap) exposedMap).clone();
         } catch (CloneNotSupportedException ex) {
             throw new IncompatibleClassChangeError("Not Cloneable.");
         }