Author: rgoers
Date: Tue Nov 23 02:27:44 2010
New Revision: 1037973

URL: http://svn.apache.org/viewvc?rev=1037973&view=rev
Log:
Replace synchronization with locking and/or concurrent objects. Add 
putFileIfAbsent method to allow callers to safely add to the cache

Modified:
    
commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/FilesCache.java
    
commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/DefaultFilesCache.java
    
commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/LRUFilesCache.java
    
commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/NullFilesCache.java
    
commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/SoftRefFilesCache.java

Modified: 
commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/FilesCache.java
URL: 
http://svn.apache.org/viewvc/commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/FilesCache.java?rev=1037973&r1=1037972&r2=1037973&view=diff
==============================================================================
--- 
commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/FilesCache.java
 (original)
+++ 
commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/FilesCache.java
 Tue Nov 23 02:27:44 2010
@@ -18,7 +18,7 @@ package org.apache.commons.vfs2;
 
 
 /**
- * The fileCache interface.
+ * The fileCache interface. Implementations of this interface are expected to 
be thread safe.
  *
  * @author <a href="http://commons.apache.org/vfs/team-list.html";>Commons VFS 
team</a>
  * @version $Revision$ $Date$
@@ -33,6 +33,14 @@ public interface FilesCache
     void putFile(final FileObject file);
 
     /**
+     * add a fileobject to the cache if it isn't already present.
+     *
+     * @param file the file
+     * @return true if the file was stored, false otherwise.
+     */
+    boolean putFileIfAbsent(final FileObject file);
+
+    /**
      * retrieve a file from the cache by its name.
      *
      * @param filesystem The FileSystem.

Modified: 
commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/DefaultFilesCache.java
URL: 
http://svn.apache.org/viewvc/commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/DefaultFilesCache.java?rev=1037973&r1=1037972&r2=1037973&view=diff
==============================================================================
--- 
commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/DefaultFilesCache.java
 (original)
+++ 
commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/DefaultFilesCache.java
 Tue Nov 23 02:27:44 2010
@@ -45,6 +45,12 @@ public class DefaultFilesCache extends A
         files.put(file.getName(), file);
     }
 
+    public boolean putFileIfAbsent(final FileObject file)
+    {
+        ConcurrentMap<FileName, FileObject> files = 
getOrCreateFilesystemCache(file.getFileSystem());
+        return files.putIfAbsent(file.getName(), file) == null;
+    }
+
     public FileObject getFile(final FileSystem filesystem, final FileName name)
     {
         Map<FileName, FileObject> files = 
getOrCreateFilesystemCache(filesystem);

Modified: 
commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/LRUFilesCache.java
URL: 
http://svn.apache.org/viewvc/commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/LRUFilesCache.java?rev=1037973&r1=1037972&r2=1037973&view=diff
==============================================================================
--- 
commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/LRUFilesCache.java
 (original)
+++ 
commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/LRUFilesCache.java
 Tue Nov 23 02:27:44 2010
@@ -16,8 +16,12 @@
  */
 package org.apache.commons.vfs2.cache;
 
-import java.util.HashMap;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import org.apache.commons.collections.map.AbstractLinkedMap;
 import org.apache.commons.collections.map.LRUMap;
@@ -47,12 +51,16 @@ public class LRUFilesCache extends Abstr
     private final Log log = LogFactory.getLog(LRUFilesCache.class);
 
     /** The FileSystem cache */
-    private final Map<FileSystem, Map<FileName, FileObject>> filesystemCache =
-          new HashMap<FileSystem, Map<FileName, FileObject>>(10);
+    private final ConcurrentMap<FileSystem, Map<FileName, FileObject>> 
filesystemCache =
+          new ConcurrentHashMap<FileSystem, Map<FileName, FileObject>>(10);
 
     /** The size of the cache */
     private final int lruSize;
 
+    private ReadWriteLock rwLock = new ReentrantReadWriteLock();
+    private Lock readLock = rwLock.readLock();
+    private Lock writeLock = rwLock.writeLock();
+
     /**
      * The file cache
      */
@@ -132,40 +140,81 @@ public class LRUFilesCache extends Abstr
 
     public void putFile(final FileObject file)
     {
-        synchronized (this)
-        {
-            Map<FileName, FileObject> files = 
getOrCreateFilesystemCache(file.getFileSystem());
+        Map<FileName, FileObject> files = 
getOrCreateFilesystemCache(file.getFileSystem());
 
+        writeLock.lock();
+        try
+        {
             // System.err.println(">>> " + files.size() + " put:" + 
file.toString());
 
             files.put(file.getName(), file);
         }
+        finally
+        {
+            writeLock.unlock();
+        }
     }
 
-    public FileObject getFile(final FileSystem filesystem, final FileName name)
+
+    public boolean putFileIfAbsent(final FileObject file)
     {
-        synchronized (this)
+        Map<FileName, FileObject> files = 
getOrCreateFilesystemCache(file.getFileSystem());
+
+        writeLock.lock();
+        try
         {
-            Map<FileName, FileObject> files = 
getOrCreateFilesystemCache(filesystem);
+            FileName name = file.getName();
 
+            // System.err.println(">>> " + files.size() + " put:" + 
file.toString());
+            if (files.containsKey(name))
+            {
+                return false;
+            }
+
+            files.put(name, file);
+            return true;
+        }
+        finally
+        {
+            writeLock.unlock();
+        }
+    }
+
+    public FileObject getFile(final FileSystem filesystem, final FileName name)
+    {
+        Map<FileName, FileObject> files = 
getOrCreateFilesystemCache(filesystem);
+
+        readLock.lock();
+        try
+        {
             // FileObject fo = (FileObject) files.get(name);
             // System.err.println(">>> " + files.size() + " get:" + 
name.toString() + " " + fo);
 
             return files.get(name);
         }
+        finally
+        {
+            readLock.unlock();
+        }
     }
 
     public void clear(final FileSystem filesystem)
     {
-        synchronized (this)
+        Map<FileName, FileObject> files = 
getOrCreateFilesystemCache(filesystem);
+
+        writeLock.lock();
+        try
         {
             // System.err.println(">>> clear fs " + filesystem);
 
-            Map<FileName, FileObject> files = 
getOrCreateFilesystemCache(filesystem);
             files.clear();
 
             filesystemCache.remove(filesystem);
         }
+        finally
+        {
+            writeLock.unlock();
+        }
     }
 
     @SuppressWarnings("unchecked") // (1)
@@ -176,8 +225,8 @@ public class LRUFilesCache extends Abstr
         {
             // System.err.println(">>> create fs " + filesystem);
 
-            files = new MyLRUMap(filesystem, lruSize); // 1
-            filesystemCache.put(filesystem, files);
+            files = new MyLRUMap(filesystem, lruSize);
+            filesystemCache.putIfAbsent(filesystem, files);
         }
 
         return files;
@@ -188,20 +237,18 @@ public class LRUFilesCache extends Abstr
     {
         super.close();
 
-        synchronized (this)
-        {
-            // System.err.println(">>> clear all");
+        // System.err.println(">>> clear all");
 
-            filesystemCache.clear();
-        }
+        filesystemCache.clear();
     }
 
     public void removeFile(final FileSystem filesystem, final FileName name)
     {
-        synchronized (this)
-        {
-            Map<?, ?> files = getOrCreateFilesystemCache(filesystem);
+        Map<?, ?> files = getOrCreateFilesystemCache(filesystem);
 
+        writeLock.lock();
+        try
+        {
             // System.err.println(">>> " + files.size() + " remove:" + 
name.toString());
 
             files.remove(name);
@@ -211,6 +258,10 @@ public class LRUFilesCache extends Abstr
                 filesystemCache.remove(filesystem);
             }
         }
+        finally
+        {
+            writeLock.unlock();
+        }
     }
 
     public void touchFile(final FileObject file)

Modified: 
commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/NullFilesCache.java
URL: 
http://svn.apache.org/viewvc/commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/NullFilesCache.java?rev=1037973&r1=1037972&r2=1037973&view=diff
==============================================================================
--- 
commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/NullFilesCache.java
 (original)
+++ 
commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/NullFilesCache.java
 Tue Nov 23 02:27:44 2010
@@ -40,6 +40,11 @@ public class NullFilesCache extends Abst
     {
     }
 
+    public boolean putFileIfAbsent(final FileObject file)
+    {
+        return false;
+    }
+
     public FileObject getFile(final FileSystem filesystem, final FileName name)
     {
         return null;

Modified: 
commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/SoftRefFilesCache.java
URL: 
http://svn.apache.org/viewvc/commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/SoftRefFilesCache.java?rev=1037973&r1=1037972&r2=1037973&view=diff
==============================================================================
--- 
commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/SoftRefFilesCache.java
 (original)
+++ 
commons/proper/vfs/trunk/core/src/main/java/org/apache/commons/vfs2/cache/SoftRefFilesCache.java
 Tue Nov 23 02:27:44 2010
@@ -31,6 +31,11 @@ import java.lang.ref.SoftReference;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
 
 /**
  * This implementation caches every file as long as it is strongly reachable by
@@ -50,13 +55,16 @@ public class SoftRefFilesCache extends A
      */
     private final Log log = LogFactory.getLog(SoftRefFilesCache.class);
 
-    private final Map<FileSystem, Map<FileName, Reference<FileObject>>> 
filesystemCache =
-          new HashMap<FileSystem, Map<FileName, Reference<FileObject>>>();
+    private final ConcurrentMap<FileSystem, Map<FileName, 
Reference<FileObject>>> filesystemCache =
+          new ConcurrentHashMap<FileSystem, Map<FileName, 
Reference<FileObject>>>();
     private final Map<Reference<FileObject>, FileSystemAndNameKey> 
refReverseMap =
           new HashMap<Reference<FileObject>, FileSystemAndNameKey>(100);
     private final ReferenceQueue<FileObject> refqueue = new 
ReferenceQueue<FileObject>();
 
-    private SoftRefReleaseThread softRefReleaseThread;
+    private AtomicReference<SoftRefReleaseThread> softRefReleaseThread = new 
AtomicReference<SoftRefReleaseThread>();
+
+    private Lock lock = new ReentrantLock();
+
 
     /**
      * This thread will listen on the ReferenceQueue and remove the entry in 
the
@@ -85,21 +93,23 @@ public class SoftRefFilesCache extends A
                         continue;
                     }
 
-                    FileSystemAndNameKey key;
-                    synchronized (refReverseMap)
+                    lock.lock();
+                    try
                     {
-                        key = refReverseMap.get(ref);
-                    }
+                        FileSystemAndNameKey key = refReverseMap.get(ref);
 
-                    if (key != null)
-                    {
-                        if (removeFile(key))
+                        if (key != null)
                         {
-                            /* This is not thread safe
-                            filesystemClose(key.getFileSystem());
-                            */
+                            if (removeFile(key))
+                            {
+                                filesystemClose(key.getFileSystem());
+                            }
                         }
                     }
+                    finally
+                    {
+                        lock.unlock();
+                    }
                 }
                 catch (InterruptedException e)
                 {
@@ -120,23 +130,31 @@ public class SoftRefFilesCache extends A
 
     private void startThread()
     {
-        if (softRefReleaseThread != null)
+        Thread thread;
+        SoftRefReleaseThread newThread;
+        do
+        {
+            newThread = null;
+            thread = softRefReleaseThread.get();
+            if (thread != null)
+            {
+                break;
+            }
+            newThread = new SoftRefReleaseThread();
+        } while (softRefReleaseThread.compareAndSet(null, newThread));
+        if (newThread != null)
         {
-            throw new IllegalStateException(
-                    
Messages.getString("vfs.impl/SoftRefReleaseThread-already-running.warn"));
+            newThread.start();
         }
-
-        softRefReleaseThread = new SoftRefReleaseThread();
-        softRefReleaseThread.start();
     }
 
     private void endThread()
     {
-        if (softRefReleaseThread != null)
+        SoftRefReleaseThread thread = softRefReleaseThread.getAndSet(null);
+        if (thread != null)
         {
-            softRefReleaseThread.requestEnd = true;
-            softRefReleaseThread.interrupt();
-            softRefReleaseThread = null;
+            thread.requestEnd = true;
+            thread.interrupt();
         }
     }
 
@@ -150,20 +168,55 @@ public class SoftRefFilesCache extends A
         Map<FileName, Reference<FileObject>> files = 
getOrCreateFilesystemCache(file.getFileSystem());
 
         Reference<FileObject> ref = createReference(file, refqueue);
-        FileSystemAndNameKey key = new FileSystemAndNameKey(file
-                .getFileSystem(), file.getName());
+        FileSystemAndNameKey key = new 
FileSystemAndNameKey(file.getFileSystem(), file.getName());
 
-        synchronized (files)
+        lock.lock();
+        try
         {
             Reference<FileObject> old = files.put(file.getName(), ref);
-            synchronized (refReverseMap)
+            if (old != null)
             {
-                if (old != null)
-                {
-                    refReverseMap.remove(old);
-                }
-                refReverseMap.put(ref, key);
+                refReverseMap.remove(old);
+            }
+            refReverseMap.put(ref, key);
+        }
+        finally
+        {
+            lock.unlock();
+        }
+    }
+
+
+    public boolean putFileIfAbsent(final FileObject file)
+    {
+        if (log.isDebugEnabled())
+        {
+            log.debug("putFile: " + file.getName());
+        }
+
+        Map<FileName, Reference<FileObject>> files = 
getOrCreateFilesystemCache(file.getFileSystem());
+
+        Reference<FileObject> ref = createReference(file, refqueue);
+        FileSystemAndNameKey key = new 
FileSystemAndNameKey(file.getFileSystem(), file.getName());
+
+        lock.lock();
+        try
+        {
+            if (files.containsKey(file.getName()) && 
files.get(file.getName()).get() != null)
+            {
+                return false;
+            }
+            Reference<FileObject> old = files.put(file.getName(), ref);
+            if (old != null)
+            {
+                refReverseMap.remove(old);
             }
+            refReverseMap.put(ref, key);
+            return true;
+        }
+        finally
+        {
+            lock.unlock();
         }
     }
 
@@ -176,7 +229,8 @@ public class SoftRefFilesCache extends A
     {
         Map<FileName, Reference<FileObject>> files = 
getOrCreateFilesystemCache(filesystem);
 
-        synchronized (files)
+        lock.lock();
+        try
         {
             Reference<FileObject> ref = files.get(name);
             if (ref == null)
@@ -191,55 +245,61 @@ public class SoftRefFilesCache extends A
             }
             return fo;
         }
+        finally
+        {
+            lock.unlock();
+        }
     }
 
     public void clear(FileSystem filesystem)
     {
         Map<FileName, Reference<FileObject>> files = 
getOrCreateFilesystemCache(filesystem);
 
-        boolean closeFilesystem;
-
-        synchronized (files)
+        lock.lock();
+        try
         {
-            synchronized (refReverseMap)
+            Iterator<FileSystemAndNameKey> iterKeys = 
refReverseMap.values().iterator();
+            while (iterKeys.hasNext())
             {
-                Iterator<FileSystemAndNameKey> iterKeys = 
refReverseMap.values().iterator();
-                while (iterKeys.hasNext())
+                FileSystemAndNameKey key = iterKeys.next();
+                if (key.getFileSystem() == filesystem)
                 {
-                    FileSystemAndNameKey key = iterKeys.next();
-                    if (key.getFileSystem() == filesystem)
-                    {
-                        iterKeys.remove();
-                        files.remove(key.getFileName());
-                    }
+                    iterKeys.remove();
+                    files.remove(key.getFileName());
                 }
+            }
 
-                closeFilesystem = files.size() < 1;
+            if (filesystemCache.size() < 1)
+            {
+                filesystemClose(filesystem);
             }
         }
-
-        if (closeFilesystem)
+        finally
         {
-            filesystemClose(filesystem);
+            lock.unlock();
         }
     }
 
+    /**
+     * Called while the lock is held
+     * @param filesystem The file system to close.
+     */
     private void filesystemClose(FileSystem filesystem)
     {
         if (log.isDebugEnabled())
         {
             log.debug("close fs: " + filesystem.getRootName());
         }
-        synchronized (filesystemCache)
+
+        filesystemCache.remove(filesystem);
+        if (filesystemCache.size() < 1)
         {
-            filesystemCache.remove(filesystem);
-            if (filesystemCache.size() < 1)
-            {
-                endThread();
-            }
+            endThread();
         }
+        /* This is not thread-safe as another thread might be opening the file 
system
         ((DefaultFileSystemManager) getContext().getFileSystemManager())
                 ._closeFileSystem(filesystem);
+         */
     }
 
     @Override
@@ -249,16 +309,17 @@ public class SoftRefFilesCache extends A
 
         endThread();
 
-        // files.clear();
-        synchronized (filesystemCache)
+        lock.lock();
+        try
         {
             filesystemCache.clear();
-        }
 
-        synchronized (refReverseMap)
-        {
             refReverseMap.clear();
         }
+        finally
+        {
+            lock.unlock();
+        }
     }
 
     public void removeFile(FileSystem filesystem, FileName name)
@@ -282,38 +343,42 @@ public class SoftRefFilesCache extends A
 
         Map<?, ?> files = getOrCreateFilesystemCache(key.getFileSystem());
 
-        synchronized (files)
+        lock.lock();
+        try
         {
             Object ref = files.remove(key.getFileName());
             if (ref != null)
             {
-                synchronized (refReverseMap)
-                {
-                    refReverseMap.remove(ref);
-                }
+                refReverseMap.remove(ref);
             }
 
             return files.size() < 1;
         }
+        finally
+        {
+            lock.unlock();
+        }
     }
 
     protected Map<FileName, Reference<FileObject>> 
getOrCreateFilesystemCache(final FileSystem filesystem)
     {
-        synchronized (filesystemCache)
+        if (filesystemCache.size() < 1)
         {
-            if (filesystemCache.size() < 1)
-            {
-                startThread();
-            }
+            startThread();
+        }
+
+        Map<FileName, Reference<FileObject>> files;
 
-            Map<FileName, Reference<FileObject>> files = 
filesystemCache.get(filesystem);
-            if (files == null)
+        do
+        {
+            files = filesystemCache.get(filesystem);
+            if (files != null)
             {
-                files = new HashMap<FileName, Reference<FileObject>>();
-                filesystemCache.put(filesystem, files);
+                break;
             }
+            files = new HashMap<FileName, Reference<FileObject>>();
+        } while (filesystemCache.putIfAbsent(filesystem, files) == null);
 
-            return files;
-        }
+        return files;
     }
 }


Reply via email to