From bed24eac4430aec88d121c1f266a98d6ff124785 Mon Sep 17 00:00:00 2001 From: Martin Stockhammer Date: Wed, 20 Sep 2017 22:34:10 +0200 Subject: [PATCH] Finally switching the file lock to java.nio --- .../filelock/DefaultFileLockManager.java | 67 +++++++------------ .../common/filelock/FileLockManager.java | 6 +- .../apache/archiva/common/filelock/Lock.java | 28 +++----- .../filelock/DefaultFileLockManagerTest.java | 40 +++++------ .../DefaultFileLockManagerTimeoutTest.java | 13 ++-- .../DefaultRepositoryProxyConnectors.java | 21 +++--- .../archiva/webdav/ArchivaDavResource.java | 4 +- 7 files changed, 75 insertions(+), 104 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 dcfeb04b3..b256fe2af 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 @@ -24,11 +24,12 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.stereotype.Service; -import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; -import java.io.RandomAccessFile; import java.nio.channels.ClosedChannelException; +import java.nio.file.Files; +import java.nio.file.NoSuchFileException; +import java.nio.file.Path; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -43,7 +44,7 @@ public class DefaultFileLockManager // TODO currently we create lock for read and write!! // the idea could be to store lock here with various clients read/write // only read could be a more simple lock and acquire a write lock means waiting the end of all reading threads - private static final ConcurrentMap lockFiles = new ConcurrentHashMap( 64 ); + private static final ConcurrentMap lockFiles = new ConcurrentHashMap( 64 ); private boolean skipLocking = true; @@ -53,7 +54,7 @@ public class DefaultFileLockManager @Override - public Lock readFileLock( File file ) + public Lock readFileLock( Path file ) throws FileLockException, FileLockTimeoutException { if ( skipLocking ) @@ -63,7 +64,11 @@ public class DefaultFileLockManager } StopWatch stopWatch = new StopWatch(); boolean acquired = false; - mkdirs( file.getParentFile() ); + try { + mkdirs(file.getParent()); + } catch (IOException e) { + throw new FileLockException("Could not create directories "+file.getParent(), e); + } Lock lock = null; @@ -116,14 +121,9 @@ public class DefaultFileLockManager lock=null; } } - catch ( FileNotFoundException e ) + catch ( FileNotFoundException | NoSuchFileException e ) { - // can happen if an other thread has deleted the file - // close RandomAccessFile!!! - if ( lock != null ) - { - closeQuietly( lock.getRandomAccessFile() ); - } + log.debug( "read Lock skip: {} try to create file", e.getMessage() ); createNewFileQuietly( file ); } @@ -143,7 +143,7 @@ public class DefaultFileLockManager @Override - public Lock writeFileLock( File file ) + public Lock writeFileLock( Path file ) throws FileLockException, FileLockTimeoutException { if ( skipLocking ) @@ -151,7 +151,11 @@ public class DefaultFileLockManager return new Lock( file ); } - mkdirs( file.getParentFile() ); + try { + mkdirs( file.getParent() ); + } catch (IOException e) { + throw new FileLockException("Could not create directory "+file.getParent(), e); + } StopWatch stopWatch = new StopWatch(); boolean acquired = false; @@ -207,14 +211,9 @@ public class DefaultFileLockManager lock=null; } } - catch ( FileNotFoundException e ) + catch ( FileNotFoundException | NoSuchFileException e ) { - // can happen if an other thread has deleted the file - // close RandomAccessFile!!! - if ( lock != null ) - { - closeQuietly( lock.getRandomAccessFile() ); - } + log.debug( "write Lock skip: {} try to create file", e.getMessage() ); createNewFileQuietly( file ); } @@ -233,28 +232,11 @@ public class DefaultFileLockManager } - private void closeQuietly( RandomAccessFile randomAccessFile ) - { - if ( randomAccessFile == null ) - { - return; - } - - try - { - randomAccessFile.close(); - } - catch ( IOException e ) - { - // ignore - } - } - - private void createNewFileQuietly( File file ) + private void createNewFileQuietly( Path file ) { try { - file.createNewFile(); + Files.createFile(file); } catch ( IOException e ) { @@ -297,9 +279,8 @@ public class DefaultFileLockManager lockFiles.clear(); } - private boolean mkdirs( File directory ) - { - return directory.mkdirs(); + private Path mkdirs( Path directory ) throws IOException { + return Files.createDirectories(directory); } @Override 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 80f6a5768..a23d97893 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 @@ -19,8 +19,8 @@ package org.apache.archiva.common.filelock; * under the License. */ -import java.io.File; import java.io.FileNotFoundException; +import java.nio.file.Path; /** * @author Olivier Lamy @@ -35,7 +35,7 @@ public interface FileLockManager * @throws FileLockException * @throws FileLockTimeoutException */ - Lock writeFileLock( File file ) + Lock writeFileLock( Path file ) throws FileLockException, FileLockTimeoutException; /** @@ -45,7 +45,7 @@ public interface FileLockManager * @throws FileLockException * @throws FileLockTimeoutException */ - Lock readFileLock( File file ) + Lock readFileLock( Path file ) throws FileLockException, FileLockTimeoutException; void release( Lock lock ) 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 17b19c0b0..ab10d7404 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 @@ -20,12 +20,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.nio.file.Path; +import java.nio.file.StandardOpenOption; import java.util.HashMap; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; @@ -37,7 +36,7 @@ import java.util.concurrent.atomic.AtomicInteger; */ public class Lock { - private File file; + private Path file; private AtomicBoolean write; @@ -45,25 +44,22 @@ public class Lock private FileLock fileLock; - private RandomAccessFile randomAccessFile; - private FileChannel fileChannel; - public Lock( File file ) + public Lock( Path file ) { this.file = file; } - public Lock( File file, boolean write ) - throws FileNotFoundException + public Lock( Path file, boolean write ) + throws IOException { this.file = file; this.write = new AtomicBoolean( write ); - randomAccessFile = new RandomAccessFile( file, write ? "rw" : "r" ); - fileChannel = randomAccessFile.getChannel(); + fileChannel = write ? FileChannel.open(file, StandardOpenOption.WRITE, StandardOpenOption.READ) : FileChannel.open(file, StandardOpenOption.READ); } - public File getFile() + public Path getFile() { return file; } @@ -73,7 +69,7 @@ public class Lock return write; } - public void setFile( File file ) + public void setFile( Path file ) { this.file = file; } @@ -122,7 +118,6 @@ public class Lock } closeQuietly( fileChannel ); - closeQuietly( randomAccessFile ); fileClients.remove( Thread.currentThread() ); @@ -144,10 +139,7 @@ public class Lock } - protected RandomAccessFile getRandomAccessFile() - { - return randomAccessFile; - } + private void closeQuietly( Closeable closeable ) { 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 adcdbc7a8..ebebf15c4 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 @@ -32,11 +32,7 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import javax.inject.Inject; import javax.inject.Named; -import java.io.File; -import java.io.FileNotFoundException; -import java.io.FileOutputStream; import java.io.IOException; -import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -66,9 +62,9 @@ public class DefaultFileLockManagerTest { FileLockManager fileLockManager; - File file = new File(System.getProperty("buildDirectory"), "foo.txt"); + Path file = Paths.get(System.getProperty("buildDirectory"), "foo.txt"); - File largeJar = new File(System.getProperty("basedir"), "src/test/cassandra-all-2.0.3.jar"); + Path largeJar = Paths.get(System.getProperty("basedir"), "src/test/cassandra-all-2.0.3.jar"); ConcurrentFileWrite(FileLockManager fileLockManager) throws IOException { @@ -104,8 +100,8 @@ public class DefaultFileLockManagerTest { logger.info("thread1"); Lock lock = fileLockManager.writeFileLock(this.file); try { - lock.getFile().delete(); - copyFile(largeJar.toPath(), lock.getFile().toPath()); + Files.deleteIfExists(lock.getFile()); + copyFile(largeJar, lock.getFile()); } finally { fileLockManager.release(lock); } @@ -124,8 +120,8 @@ public class DefaultFileLockManagerTest { logger.info("thread2"); Lock lock = fileLockManager.writeFileLock(this.file); try { - lock.getFile().delete(); - copyFile(largeJar.toPath(), lock.getFile().toPath()); + Files.deleteIfExists(lock.getFile()); + copyFile(largeJar, lock.getFile()); } finally { fileLockManager.release(lock); } @@ -147,7 +143,7 @@ public class DefaultFileLockManagerTest { Path outFile = null; try { outFile = Files.createTempFile("foo", ".jar"); - Files.copy(Paths.get(lock.getFile().getPath()), + Files.copy(lock.getFile(), Files.newOutputStream(outFile)); } finally { fileLockManager.release(lock); @@ -169,8 +165,8 @@ public class DefaultFileLockManagerTest { logger.info("thread4"); Lock lock = fileLockManager.writeFileLock(this.file); try { - lock.getFile().delete(); - copyFile(largeJar.toPath(), lock.getFile().toPath()); + Files.deleteIfExists(lock.getFile()); + copyFile(largeJar, lock.getFile()); } finally { fileLockManager.release(lock); } @@ -190,8 +186,8 @@ public class DefaultFileLockManagerTest { logger.info("thread5"); Lock lock = fileLockManager.writeFileLock(this.file); try { - lock.getFile().delete(); - copyFile(largeJar.toPath(), lock.getFile().toPath()); + Files.deleteIfExists(lock.getFile()); + copyFile(largeJar, lock.getFile()); } finally { fileLockManager.release(lock); } @@ -213,7 +209,7 @@ public class DefaultFileLockManagerTest { Path outFile = null; try { outFile = Files.createTempFile("foo", ".jar"); - Files.copy(lock.getFile().toPath(), Files.newOutputStream( outFile )); + Files.copy(lock.getFile(), Files.newOutputStream( outFile )); } finally { fileLockManager.release(lock); if (outFile!=null) Files.delete( outFile ); @@ -234,8 +230,8 @@ public class DefaultFileLockManagerTest { logger.info("thread7"); Lock lock = fileLockManager.writeFileLock(this.file); try { - lock.getFile().delete(); - copyFile(largeJar.toPath(), lock.getFile().toPath()); + Files.deleteIfExists(lock.getFile()); + copyFile(largeJar, lock.getFile()); } finally { fileLockManager.release(lock); } @@ -257,7 +253,7 @@ public class DefaultFileLockManagerTest { Path outFile = null; try { outFile = Files.createTempFile("foo", ".jar"); - Files.copy(lock.getFile().toPath(), Files.newOutputStream( outFile )); + Files.copy(lock.getFile(), Files.newOutputStream( outFile )); } finally { fileLockManager.release(lock); if (outFile!=null) Files.delete( outFile ); @@ -278,8 +274,8 @@ public class DefaultFileLockManagerTest { logger.info("thread9"); Lock lock = fileLockManager.writeFileLock(this.file); try { - lock.getFile().delete(); - copyFile(largeJar.toPath(), lock.getFile().toPath()); + Files.deleteIfExists(lock.getFile()); + copyFile(largeJar, lock.getFile()); } finally { fileLockManager.release(lock); } @@ -300,7 +296,7 @@ public class DefaultFileLockManagerTest { Path outFile = null; try { outFile = Files.createTempFile("foo", ".jar"); - Files.copy(lock.getFile().toPath(), Files.newOutputStream( outFile )); + Files.copy(lock.getFile(), Files.newOutputStream( outFile )); } finally { fileLockManager.release(lock); if (outFile!=null) Files.delete(outFile); diff --git a/archiva-modules/archiva-base/archiva-filelock/src/test/java/org/apache/archiva/common/filelock/DefaultFileLockManagerTimeoutTest.java b/archiva-modules/archiva-base/archiva-filelock/src/test/java/org/apache/archiva/common/filelock/DefaultFileLockManagerTimeoutTest.java index 514dfde02..6ed069a88 100644 --- a/archiva-modules/archiva-base/archiva-filelock/src/test/java/org/apache/archiva/common/filelock/DefaultFileLockManagerTimeoutTest.java +++ b/archiva-modules/archiva-base/archiva-filelock/src/test/java/org/apache/archiva/common/filelock/DefaultFileLockManagerTimeoutTest.java @@ -29,11 +29,8 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import javax.inject.Inject; import javax.inject.Named; -import java.io.File; import java.io.IOException; -import java.nio.file.FileSystemException; -import java.nio.file.Files; -import java.nio.file.StandardCopyOption; +import java.nio.file.*; /** * @author Olivier Lamy @@ -65,16 +62,16 @@ public class DefaultFileLockManagerTimeoutTest { try { - File file = new File(System.getProperty("buildDirectory"), "foo.txt"); + Path file = Paths.get(System.getProperty("buildDirectory"), "foo.txt"); - Files.deleteIfExists(file.toPath()); + Files.deleteIfExists(file); - File largeJar = new File(System.getProperty("basedir"), "src/test/cassandra-all-2.0.3.jar"); + Path largeJar = Paths.get(System.getProperty("basedir"), "src/test/cassandra-all-2.0.3.jar"); Lock lock = fileLockManager.writeFileLock(file); try { - Files.copy(largeJar.toPath(), lock.getFile().toPath(), StandardCopyOption.REPLACE_EXISTING); + Files.copy(largeJar, lock.getFile(), StandardCopyOption.REPLACE_EXISTING); } catch (IOException e) { logger.warn("Copy failed {}", e.getMessage()); // On windows a FileSystemException is thrown diff --git a/archiva-modules/archiva-base/archiva-proxy/src/main/java/org/apache/archiva/proxy/DefaultRepositoryProxyConnectors.java b/archiva-modules/archiva-base/archiva-proxy/src/main/java/org/apache/archiva/proxy/DefaultRepositoryProxyConnectors.java index f04fe192e..4266bf734 100644 --- a/archiva-modules/archiva-base/archiva-proxy/src/main/java/org/apache/archiva/proxy/DefaultRepositoryProxyConnectors.java +++ b/archiva-modules/archiva-base/archiva-proxy/src/main/java/org/apache/archiva/proxy/DefaultRepositoryProxyConnectors.java @@ -1089,17 +1089,22 @@ public class DefaultRepositoryProxyConnectors Lock lock; try { - lock = fileLockManager.writeFileLock( target.toFile() ); - if ( lock.getFile().exists() && !lock.getFile().delete() ) - { + lock = fileLockManager.writeFileLock( target ); + try { + Files.deleteIfExists(lock.getFile()); + } catch (IOException e) { throw new ProxyException( "Unable to overwrite existing target file: " + target.toAbsolutePath() ); } - lock.getFile().getParentFile().mkdirs(); + try { + Files.createDirectories(lock.getFile().getParent()); + } catch (IOException e) { + throw new ProxyException("Unable to create parent directory "+lock.getFile().getParent()); + } try { - Files.move(temp, lock.getFile().toPath() ); + Files.move(temp, lock.getFile() ); } catch ( IOException e ) { @@ -1107,14 +1112,14 @@ public class DefaultRepositoryProxyConnectors try { - Files.copy( temp, lock.getFile().toPath() ); + Files.copy( temp, lock.getFile()); } catch ( IOException e2 ) { - if ( lock.getFile().exists() ) + if ( Files.exists(lock.getFile()) ) { log.debug( "Tried to copy file {} to {} but file with this name already exists.", - temp.getFileName(), lock.getFile().getAbsolutePath() ); + temp.getFileName(), lock.getFile().toAbsolutePath() ); } else { diff --git a/archiva-modules/archiva-web/archiva-webdav/src/main/java/org/apache/archiva/webdav/ArchivaDavResource.java b/archiva-modules/archiva-web/archiva-webdav/src/main/java/org/apache/archiva/webdav/ArchivaDavResource.java index 412433197..b152e8980 100644 --- a/archiva-modules/archiva-web/archiva-webdav/src/main/java/org/apache/archiva/webdav/ArchivaDavResource.java +++ b/archiva-modules/archiva-web/archiva-webdav/src/main/java/org/apache/archiva/webdav/ArchivaDavResource.java @@ -212,8 +212,8 @@ public class ArchivaDavResource { if ( !isCollection() && outputContext.hasStream() ) { - Lock lock = fileLockManager.readFileLock( localResource.toFile() ); - try (InputStream is = Files.newInputStream( lock.getFile().toPath() )) + Lock lock = fileLockManager.readFileLock( localResource ); + try (InputStream is = Files.newInputStream( lock.getFile())) { IOUtils.copy( is, outputContext.getOutputStream() ); }