cleanup this file locking library

git-svn-id: https://svn.apache.org/repos/asf/archiva/trunk@1550556 13f79535-47bb-0310-9956-ffa450edef68

4 files changed, 190 insertions, 113 deletions

diff --git a/archiva-modules/archiva-base/archiva-filelock/src/main/java/org/apache/archiva/common/filelock/DefaultFileLockManager.java b/archiva-modules/archiva-base/archiva-filelock/src/main/java/org/apache/archiva/common/filelock/DefaultFileLockManager.java
index 795760672..145b3924a 100644
--- a/archiva-modules/archiva-base/archiva-filelock/src/main/java/org/apache/archiva/common/filelock/DefaultFileLockManager.java
+++ b/archiva-modules/archiva-base/archiva-filelock/src/main/java/org/apache/archiva/common/filelock/DefaultFileLockManager.java
@@ -3,11 +3,11 @@ package org.apache.archiva.common.filelock;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.stereotype.Service;
+import org.springframework.util.StopWatch;
 
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.IOException;
-
-import java.nio.channels.OverlappingFileLockException;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 
@@ -24,121 +24,98 @@ public class DefaultFileLockManager
 
     private Logger log = LoggerFactory.getLogger( getClass() );
 
+    private int timeout = 0;
+
     @Override
     public Lock readFileLock( File file )
-        throws FileLockException
+        throws FileLockException, FileNotFoundException
     {
         if ( skipLocking )
         {
-            try
-            {
-                return new Lock( file, false );
-            }
-            catch ( IOException e )
-            {
-                throw new FileLockException( e.getMessage(), e );
-            }
+            return new Lock( file );
+
         }
-        Lock lock = lockFiles.get( file );
-        if ( lock == null )
+        StopWatch stopWatch = new StopWatch();
+        boolean acquired = false;
+
+        Lock lock = new Lock( file, false );
+
+        stopWatch.start();
+
+        while ( !acquired )
         {
-            try
+            if ( timeout > 0 )
             {
-                lock = new Lock( file, false );
-                Lock current = lockFiles.putIfAbsent( file, lock );
-                if ( current != null )
+                long delta = stopWatch.getTotalTimeMillis();
+                if ( delta > timeout )
                 {
-                    lock = current;
+                    log.warn( "Cannot acquire read lock within {} millis. Will skip the file: {}", timeout, file );
+                    // we could not get the lock within the timeout period, so return null
+                    return null;
                 }
-                return lock;
             }
-            catch ( IOException e )
+            try
             {
-                throw new FileLockException( e.getMessage(), e );
+                lock.openLock( false, timeout > 0 );
+                acquired = true;
             }
-            catch ( OverlappingFileLockException e )
+            catch ( IOException e )
             {
-                log.debug( "OverlappingFileLockException: {}", e.getMessage() );
-                if ( lock == null )
-                {
-                    lock = lockFiles.get( file );
-                }
+                throw new FileLockException( e.getMessage(), e );
             }
-        }
-        // FIXME add a timeout on getting that!!!
-        while ( true )
-        {
-            log.debug( "wait read lock" );
-            synchronized ( lock )
+            catch ( IllegalStateException e )
             {
-                if ( lock.getFileLock().isShared() || !lock.getFileLock().isValid() )
-                {
-                    lock.addFileClient( Thread.currentThread() );
-                    return lock;
-                }
+                log.debug( "openLock {}:{}", e.getClass(), e.getMessage() );
             }
         }
-        //return lock;
+        return lock;
     }
 
+
     @Override
     public Lock writeFileLock( File file )
-        throws FileLockException
+        throws FileLockException, FileNotFoundException
     {
-        try
+        if ( skipLocking )
         {
-            if ( skipLocking )
-            {
-                return new Lock( file, true );
-            }
+            return new Lock( file );
+        }
+
+        StopWatch stopWatch = new StopWatch();
+        boolean acquired = false;
 
-            // FIXME add a timeout on getting that!!!
-            while ( true )
+        Lock lock = new Lock( file, true );
+
+        stopWatch.start();
+
+        while ( !acquired )
+        {
+            if ( timeout > 0 )
             {
-                Lock lock = lockFiles.get( file );
-                log.debug( "wait write lock" );
-                if ( lock != null )
-                {
-                    synchronized ( lock )
-                    {
-                        if ( lock.getFileLock().isValid() || lock.getFileClients().size() > 0 )
-                        {
-                            continue;
-                        }
-                        return lock;
-                    }
-                }
-                else
+                long delta = stopWatch.getTotalTimeMillis();
+                if ( delta > timeout )
                 {
-                    try
-                    {
-                        lock = new Lock( file, true );
-                    }
-                    catch ( OverlappingFileLockException e )
-                    {
-                        log.debug( "OverlappingFileLockException: {}", e.getMessage() );
-                        if ( lock == null )
-                        {
-                            lock = lockFiles.get( file );
-                        }
-
-                        lock = lockFiles.get( file );
-                        log.debug( "OverlappingFileLockException get: {}", lock );
-                    }
-                    Lock current = lockFiles.putIfAbsent( file, lock );
-                    if ( current != null )
-                    {
-                        lock = current;
-                    }
-                    return lock;
+                    log.warn( "Cannot acquire read lock within {} millis. Will skip the file: {}", timeout, file );
+                    // we could not get the lock within the timeout period, so return null
+                    return null;
                 }
             }
-
-        }
-        catch ( IOException e )
-        {
-            throw new FileLockException( e.getMessage(), e );
+            try
+            {
+                lock.openLock( true, timeout > 0 );
+                acquired = true;
+            }
+            catch ( IOException e )
+            {
+                throw new FileLockException( e.getMessage(), e );
+            }
+            catch ( IllegalStateException e )
+            {
+                log.debug( "openLock {}:{}", e.getClass(), e.getMessage() );
+            }
         }
+        return lock;
+
     }
 
     @Override
@@ -156,22 +133,23 @@ public class DefaultFileLockManager
         }
         try
         {
-            if ( lock.isWrite().get() )
-            {
-                lock.getFileLock().release();
-            }
-            synchronized ( lock )
-            {
-                lock.close();
-                if ( lock.getFileClients().size() < 1 )
-                {
-                    lockFiles.remove( lock.getFile() );
-                }
-            }
+            lock.close();
         }
         catch ( IOException e )
         {
             throw new FileLockException( e.getMessage(), e );
         }
     }
+
+    public int getTimeout()
+    {
+        return timeout;
+    }
+
+    public void setTimeout( int timeout )
+    {
+        this.timeout = timeout;
+    }
+
+
 }
diff --git a/archiva-modules/archiva-base/archiva-filelock/src/main/java/org/apache/archiva/common/filelock/FileLockManager.java b/archiva-modules/archiva-base/archiva-filelock/src/main/java/org/apache/archiva/common/filelock/FileLockManager.java
index 5f3832634..e02e0c159 100644
--- a/archiva-modules/archiva-base/archiva-filelock/src/main/java/org/apache/archiva/common/filelock/FileLockManager.java
+++ b/archiva-modules/archiva-base/archiva-filelock/src/main/java/org/apache/archiva/common/filelock/FileLockManager.java
@@ -1,6 +1,7 @@
 package org.apache.archiva.common.filelock;
 
 import java.io.File;
+import java.io.FileNotFoundException;
 
 /**
  * @author Olivier Lamy
@@ -8,11 +9,15 @@ import java.io.File;
 public interface FileLockManager
 {
     Lock writeFileLock( File file )
-        throws FileLockException;
+        throws FileLockException, FileNotFoundException;
 
     Lock readFileLock( File file )
-        throws FileLockException;
+        throws FileLockException, FileNotFoundException;
 
     void release( Lock lock )
-        throws FileLockException;
+        throws FileLockException, FileNotFoundException;
+
+    int getTimeout();
+
+    void setTimeout( int timeout );
 }
diff --git a/archiva-modules/archiva-base/archiva-filelock/src/main/java/org/apache/archiva/common/filelock/Lock.java b/archiva-modules/archiva-base/archiva-filelock/src/main/java/org/apache/archiva/common/filelock/Lock.java
index 6baabbc05..422bde71f 100644
--- a/archiva-modules/archiva-base/archiva-filelock/src/main/java/org/apache/archiva/common/filelock/Lock.java
+++ b/archiva-modules/archiva-base/archiva-filelock/src/main/java/org/apache/archiva/common/filelock/Lock.java
@@ -1,9 +1,11 @@
 package org.apache.archiva.common.filelock;
 
+import java.io.Closeable;
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.RandomAccessFile;
+import java.nio.channels.FileChannel;
 import java.nio.channels.FileLock;
 import java.util.HashMap;
 import java.util.Map;
@@ -23,12 +25,22 @@ public class Lock
 
     private FileLock fileLock;
 
+    private RandomAccessFile randomAccessFile;
+
+    private FileChannel fileChannel;
+
+    public Lock( File file )
+    {
+        this.file = file;
+    }
+
     public Lock( File file, boolean write )
-        throws FileNotFoundException, IOException
+        throws FileNotFoundException
     {
         this.file = file;
         this.write = new AtomicBoolean( write );
-        this.openLock( write );
+        randomAccessFile = new RandomAccessFile( file, write ? "rw" : "r" );
+        fileChannel = randomAccessFile.getChannel();
     }
 
     public File getFile()
@@ -51,14 +63,14 @@ public class Lock
         this.write.set( write );
     }
 
-    public FileLock getFileLock()
+    public boolean isShared()
     {
-        return fileLock;
+        return this.fileLock.isValid() && this.fileLock.isShared();
     }
 
-    public void setFileLock( FileLock fileLock )
+    public boolean isValid()
     {
-        this.fileLock = fileLock;
+        return this.fileLock.isValid();
     }
 
     public Map<Thread, AtomicInteger> getFileClients()
@@ -79,21 +91,53 @@ public class Lock
     protected void close()
         throws IOException
     {
-        if ( this.write.get() )
+        IOException ioException = null;
+        try
         {
             this.fileLock.release();
-            fileClients.remove( Thread.currentThread() );
         }
+        catch ( IOException e )
+        {
+            ioException = e;
+        }
+
+        closeQuietly( fileChannel );
+        closeQuietly( randomAccessFile );
+
+        fileClients.remove( Thread.currentThread() );
+
+        if ( ioException != null )
+        {
+            throw ioException;
+        }
+
     }
 
-    public void openLock( boolean write )
+    protected void openLock( boolean write, boolean timeout )
         throws IOException
     {
         fileClients.put( Thread.currentThread(), new AtomicInteger( 1 ) );
-        RandomAccessFile raf = new RandomAccessFile( file, write ? "rw" : "r" );
-        this.fileLock = raf.getChannel().lock( 1, 1, !write );
+
+        this.fileLock = timeout
+            ? fileChannel.tryLock( 0L, Long.MAX_VALUE, write ? false : true )
+            : fileChannel.lock( 0L, Long.MAX_VALUE, write ? false : true );
+
+    }
+
+
+    private void closeQuietly( Closeable closeable )
+    {
+        try
+        {
+            closeable.close();
+        }
+        catch ( IOException e )
+        {
+            // ignore
+        }
     }
 
+
     @Override
     public String toString()
     {
diff --git a/archiva-modules/archiva-base/archiva-filelock/src/test/java/org/apache/archiva/common/filelock/DefaultFileLockManagerTest.java b/archiva-modules/archiva-base/archiva-filelock/src/test/java/org/apache/archiva/common/filelock/DefaultFileLockManagerTest.java
index 86fb24dd3..dac83aa8e 100644
--- a/archiva-modules/archiva-base/archiva-filelock/src/test/java/org/apache/archiva/common/filelock/DefaultFileLockManagerTest.java
+++ b/archiva-modules/archiva-base/archiva-filelock/src/test/java/org/apache/archiva/common/filelock/DefaultFileLockManagerTest.java
@@ -4,6 +4,7 @@ import edu.umd.cs.mtc.MultithreadedTestCase;
 import edu.umd.cs.mtc.TestFramework;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
+import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.slf4j.Logger;
@@ -17,6 +18,7 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.util.concurrent.atomic.AtomicInteger;
 
 /**
  * @author Olivier Lamy
@@ -36,6 +38,9 @@ public class DefaultFileLockManagerTest
         extends MultithreadedTestCase
     {
 
+
+        AtomicInteger success = new AtomicInteger( 0 );
+
         FileLockManager fileLockManager;
 
         File file = new File( System.getProperty( "buildDirectory" ), "foo.txt" );
@@ -71,6 +76,7 @@ public class DefaultFileLockManagerTest
                 fileLockManager.release( lock );
             }
             logger.info( "thread1 ok" );
+            success.incrementAndGet();
         }
 
         public void thread2()
@@ -88,6 +94,7 @@ public class DefaultFileLockManagerTest
                 fileLockManager.release( lock );
             }
             logger.info( "thread2 ok" );
+            success.incrementAndGet();
         }
 
         public void thread3()
@@ -105,6 +112,7 @@ public class DefaultFileLockManagerTest
                 fileLockManager.release( lock );
             }
             logger.info( "thread3 ok" );
+            success.incrementAndGet();
         }
 
         public void thread4()
@@ -122,6 +130,7 @@ public class DefaultFileLockManagerTest
                 fileLockManager.release( lock );
             }
             logger.info( "thread4 ok" );
+            success.incrementAndGet();
         }
 
         public void thread5()
@@ -139,6 +148,7 @@ public class DefaultFileLockManagerTest
                 fileLockManager.release( lock );
             }
             logger.info( "thread5 ok" );
+            success.incrementAndGet();
         }
 
         public void thread6()
@@ -156,6 +166,7 @@ public class DefaultFileLockManagerTest
                 fileLockManager.release( lock );
             }
             logger.info( "thread6 ok" );
+            success.incrementAndGet();
         }
 
         public void thread7()
@@ -173,6 +184,7 @@ public class DefaultFileLockManagerTest
                 fileLockManager.release( lock );
             }
             logger.info( "thread7 ok" );
+            success.incrementAndGet();
         }
 
         public void thread8()
@@ -190,8 +202,44 @@ public class DefaultFileLockManagerTest
                 fileLockManager.release( lock );
             }
             logger.info( "thread8 ok" );
+            success.incrementAndGet();
         }
 
+        public void thread9()
+            throws FileLockException, IOException
+        {
+            logger.info( "thread7" );
+            Lock lock = fileLockManager.writeFileLock( this.file );
+            try
+            {
+                lock.getFile().delete();
+                FileUtils.copyFile( largeJar, lock.getFile() );
+            }
+            finally
+            {
+                fileLockManager.release( lock );
+            }
+            logger.info( "thread9 ok" );
+            success.incrementAndGet();
+        }
+
+        public void thread10()
+            throws FileLockException, IOException
+        {
+            logger.info( "thread10" );
+            Lock lock = fileLockManager.readFileLock( this.file );
+            try
+            {
+                IOUtils.copy( new FileInputStream( lock.getFile() ),
+                              new FileOutputStream( File.createTempFile( "foo", ".jar" ) ) );
+            }
+            finally
+            {
+                fileLockManager.release( lock );
+            }
+            logger.info( "thread8 ok" );
+            success.incrementAndGet();
+        }
 
 
     }
@@ -203,6 +251,8 @@ public class DefaultFileLockManagerTest
         ConcurentFileWrite concurentFileWrite = new ConcurentFileWrite( fileLockManager );
         //concurentFileWrite.setTrace( true );
         TestFramework.runOnce( concurentFileWrite );
+        logger.info( "success: {}", concurentFileWrite.success );
+        Assert.assertEquals( 10, concurentFileWrite.success.intValue() );
     }
 
 }