Cleanup locking of container collections.

git-svn-id: https://svn.apache.org/repos/asf/commons/proper/vfs/trunk@1705085 13f79535-47bb-0310-9956-ffa450edef68

core/src/main/java/org/apache/commons/vfs2/provider/AbstractFileProvider.java

@@ -35,11 +35,16 @@ public abstract class AbstractFileProvider
     extends AbstractVfsContainer
     implements FileProvider
 {
+    private static final AbstractFileSystem[] EMPTY_ABSTRACTFILESYSTEMS = new AbstractFileSystem[0];
+
     /**
-     * The cached file systems.  This is a mapping from root URI to
-     * FileSystem object.
+     * The cached file systems.
+     * <p>
+     * This is a mapping from {@link FileSystemKey} (root URI and options)
+     * to {@link FileSystem}.
      */
-    private final Map<FileSystemKey, FileSystem> fileSystems = new TreeMap<FileSystemKey, FileSystem>();
+    private final Map<FileSystemKey, FileSystem> fileSystems
+            = new TreeMap<FileSystemKey, FileSystem>(); // @GuardedBy("self")
 
     private FileNameParser parser;
 
@@ -64,7 +69,7 @@ protected void setFileNameParser(final FileNameParser parser)
     @Override
     public void close()
     {
-        synchronized (this)
+        synchronized (fileSystems)
         {
             fileSystems.clear();
         }
@@ -99,13 +104,13 @@ public FileObject createFileSystem(final String scheme, final FileObject file, f
     protected void addFileSystem(final Comparable<?> key, final FileSystem fs)
         throws FileSystemException
     {
-        // Add to the cache
+        // Add to the container and initialize
         addComponent(fs);
 
         final FileSystemKey treeKey = new FileSystemKey(key, fs.getFileSystemOptions());
         ((AbstractFileSystem) fs).setCacheKey(treeKey);
 
-        synchronized (this)
+        synchronized (fileSystems)
         {
             fileSystems.put(treeKey, fs);
         }
@@ -122,7 +127,7 @@ protected FileSystem findFileSystem(final Comparable<?> key, final FileSystemOpt
     {
         final FileSystemKey treeKey = new FileSystemKey(key, fileSystemProps);
 
-        synchronized (this)
+        synchronized (fileSystems)
         {
             return fileSystems.get(treeKey);
         }
@@ -143,14 +148,16 @@ public FileSystemConfigBuilder getConfigBuilder()
      */
     public void freeUnusedResources()
     {
-        Object[] item;
-        synchronized (this)
+        AbstractFileSystem[] abstractFileSystems;
+        synchronized (fileSystems)
         {
-            item = fileSystems.values().toArray();
+            // create snapshot under lock
+            abstractFileSystems = fileSystems.values().toArray(EMPTY_ABSTRACTFILESYSTEMS);
         }
-        for (final Object element : item)
+
+        // process snapshot outside lock
+        for (final AbstractFileSystem fs : abstractFileSystems)
         {
-            final AbstractFileSystem fs = (AbstractFileSystem) element;
             if (fs.isReleaseable())
             {
                 fs.closeCommunicationLink();
@@ -166,11 +173,12 @@ public void closeFileSystem(final FileSystem filesystem)
     {
         final AbstractFileSystem fs = (AbstractFileSystem) filesystem;
 
-        synchronized (this)
+        final FileSystemKey key = fs.getCacheKey();
+        if (key != null)
         {
-            if (fs.getCacheKey() != null)
+            synchronized (fileSystems)
             {
-                fileSystems.remove(fs.getCacheKey());
+                fileSystems.remove(key);
             }
         }
 

core/src/main/java/org/apache/commons/vfs2/provider/AbstractVfsContainer.java

@@ -29,7 +29,8 @@ public abstract class AbstractVfsContainer
     /**
      * The components contained by this component.
      */
-    private final ArrayList<Object> components = new ArrayList<Object>();
+    private final ArrayList<Object> components
+            = new ArrayList<Object>(); // @GuardedBy("self")
 
     /**
      * Adds a sub-component to this component.
@@ -42,20 +43,23 @@ public abstract class AbstractVfsContainer
     protected void addComponent(final Object component)
         throws FileSystemException
     {
-        if (!components.contains(component))
+        synchronized (components)
         {
-            // Initialise
-            if (component instanceof VfsComponent)
+            if (!components.contains(component))
             {
-                final VfsComponent vfsComponent = (VfsComponent) component;
-                vfsComponent.setLogger(getLogger());
-                vfsComponent.setContext(getContext());
-                vfsComponent.init();
-            }
+                // Initialise
+                if (component instanceof VfsComponent)
+                {
+                    final VfsComponent vfsComponent = (VfsComponent) component;
+                    vfsComponent.setLogger(getLogger());
+                    vfsComponent.setContext(getContext());
+                    vfsComponent.init();
+                }
 
-            // Keep track of component, to close it later
-            components.add(component);
-        }
+                // Keep track of component, to close it later
+                components.add(component);
+            }
+        } // synchronized
     }
 
     /**
@@ -65,7 +69,11 @@ protected void addComponent(final Object component)
      */
     protected void removeComponent(final Object component)
     {
-        components.remove(component);
+        synchronized (components)
+        {
+            // multiple instances should not happen
+            components.remove(component);
+        }
     }
 
     /**
@@ -74,17 +82,21 @@ protected void removeComponent(final Object component)
     @Override
     public void close()
     {
+        final Object[] toclose;
+        synchronized (components)
+        {
+            toclose = components.toArray();
+            components.clear();
+        }
+
         // Close all components
-        final int count = components.size();
-        for (int i = 0; i < count; i++)
+        for (Object component : toclose)
         {
-            final Object component = components.get(i);
             if (component instanceof VfsComponent)
             {
                 final VfsComponent vfsComponent = (VfsComponent) component;
                 vfsComponent.close();
             }
         }
-        components.clear();
     }
 }