From 875890880ae2fafa855b7326ea276cab92e51e42 Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Thu, 12 Dec 2013 22:36:40 +0000 Subject: [PATCH] cleanup this file locking library git-svn-id: https://svn.apache.org/repos/asf/archiva/trunk@1550556 13f79535-47bb-0310-9956-ffa450edef68 --- .../filelock/DefaultFileLockManager.java | 176 ++++++++---------- .../common/filelock/FileLockManager.java | 11 +- .../apache/archiva/common/filelock/Lock.java | 66 +++++-- .../filelock/DefaultFileLockManagerTest.java | 50 +++++ 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 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() ); } } -- 2.39.5