Author: ecki Date: Thu Sep 24 15:47:40 2015 New Revision: 1705085 URL: http://svn.apache.org/viewvc?rev=1705085&view=rev Log: Cleanup locking of container collections.
Modified: commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/provider/AbstractFileProvider.java commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/provider/AbstractVfsContainer.java Modified: commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/provider/AbstractFileProvider.java URL: http://svn.apache.org/viewvc/commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/provider/AbstractFileProvider.java?rev=1705085&r1=1705084&r2=1705085&view=diff ============================================================================== --- commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/provider/AbstractFileProvider.java (original) +++ commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/provider/AbstractFileProvider.java Thu Sep 24 15:47:40 2015 @@ -35,11 +35,16 @@ public abstract class AbstractFileProvid 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 @@ public abstract class AbstractFileProvid @Override public void close() { - synchronized (this) + synchronized (fileSystems) { fileSystems.clear(); } @@ -99,13 +104,13 @@ public abstract class AbstractFileProvid 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 @@ public abstract class AbstractFileProvid { final FileSystemKey treeKey = new FileSystemKey(key, fileSystemProps); - synchronized (this) + synchronized (fileSystems) { return fileSystems.get(treeKey); } @@ -143,14 +148,16 @@ public abstract class AbstractFileProvid */ 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 abstract class AbstractFileProvid { 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); } } Modified: commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/provider/AbstractVfsContainer.java URL: http://svn.apache.org/viewvc/commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/provider/AbstractVfsContainer.java?rev=1705085&r1=1705084&r2=1705085&view=diff ============================================================================== --- commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/provider/AbstractVfsContainer.java (original) +++ commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/provider/AbstractVfsContainer.java Thu Sep 24 15:47:40 2015 @@ -29,7 +29,8 @@ public abstract class AbstractVfsContain /** * 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 AbstractVfsContain 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 @@ public abstract class AbstractVfsContain */ protected void removeComponent(final Object component) { - components.remove(component); + synchronized (components) + { + // multiple instances should not happen + components.remove(component); + } } /** @@ -74,17 +82,21 @@ public abstract class AbstractVfsContain @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(); } }