ant-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From gscok...@apache.org
Subject svn commit: r687325 - /ant/core/trunk/src/main/org/apache/tools/ant/PropertyHelper.java
Date Wed, 20 Aug 2008 13:28:52 GMT
Author: gscokart
Date: Wed Aug 20 06:28:52 2008
New Revision: 687325

URL: http://svn.apache.org/viewvc?rev=687325&view=rev
Log:
Thread safety fix (list of delegates were modified and copied concurrently without common
lock).  The hashtable is thread safe and not published outside the class, so no need to copy
it (synchronize non atomic modification is enought).  However, the list contained in the delegates
hashtable are published and should thus be copied.  I put the copy in the add method so that
the getDelegates doesn't need to be synchronized.

Modified:
    ant/core/trunk/src/main/org/apache/tools/ant/PropertyHelper.java

Modified: ant/core/trunk/src/main/org/apache/tools/ant/PropertyHelper.java
URL: http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/ant/PropertyHelper.java?rev=687325&r1=687324&r2=687325&view=diff
==============================================================================
--- ant/core/trunk/src/main/org/apache/tools/ant/PropertyHelper.java (original)
+++ ant/core/trunk/src/main/org/apache/tools/ant/PropertyHelper.java Wed Aug 20 06:28:52 2008
@@ -177,7 +177,7 @@
 
     private Project project;
     private PropertyHelper next;
-    private volatile Hashtable delegates = new Hashtable();
+    private Hashtable delegates = new Hashtable();
 
     /** Project properties map (usually String to String). */
     private Hashtable properties = new Hashtable();
@@ -924,35 +924,33 @@
      * @since Ant 1.8
      */
     public void add(Delegate delegate) {
-        synchronized (Delegate.class) {
-            Hashtable newDelegates = (Hashtable) delegates.clone();
+        synchronized (delegates) {
             for (Iterator iter = getDelegateInterfaces(delegate).iterator(); iter.hasNext();)
{
                 Object key = iter.next();
-                List list = (List) newDelegates.get(key);
+                List list = (List) delegates.get(key);
                 if (list == null) {
                     list = new ArrayList();
-                    newDelegates.put(key, list);
-                }
-                if (list.contains(delegate)) {
+                } else {
+                    list = new ArrayList(list);
                     list.remove(delegate);
                 }
                 list.add(0, delegate);
+                delegates.put(key, Collections.unmodifiableList(list));
             }
-            delegates = newDelegates;
         }
     }
 
     /**
      * Get the Collection of delegates of the specified type.
-     * @param type delegate type.
+     * 
+     * @param type
+     *            delegate type.
      * @return Collection.
      * @since Ant 1.8
      */
     protected List getDelegates(Class type) {
-        Hashtable curDelegates = delegates;
-        return curDelegates.containsKey(type)
-            ? (List) new ArrayList((List) curDelegates.get(type))
-            : Collections.EMPTY_LIST;
+        List r = (List) delegates.get(type);
+        return r == null ? Collections.EMPTY_LIST : r;
     }
 
     /**



Mime
View raw message