Introduce FileLockService.Locking to leverage try-with-resource-blocks

This allows to simplify the usage of the FileLocker/FileLockingService
to:

try (var locking = fileLockService.lock(fileToProtect)) {
  // do something
}

tycho-api/src/main/java/org/eclipse/tycho/FileLockService.java

@@ -13,6 +13,7 @@
 
 package org.eclipse.tycho;
 
+import java.io.Closeable;
 import java.io.File;
 
 /**
@@ -21,10 +22,16 @@
 public interface FileLockService {
 
     /**
-     * Get a locker object which can be used to protect read/write access from multiple processes on
-     * the given file. Locking is advisory only, i.e. all processes must use the same locking
-     * mechanism.
+     * Locks the given file to protect read/write access from multiple processes on it. Locking is
+     * advisory only, i.e. all processes must use the same locking mechanism.
      */
-    public FileLocker getFileLocker(File file);
+    default Closeable lock(File file) {
+        return lock(file, 10000L);
+    }
 
+    /**
+     * Locks the given file to protect read/write access from multiple processes on it. Locking is
+     * advisory only, i.e. all processes must use the same locking mechanism.
+     */
+    Closeable lock(File file, long timeout);
 }

tycho-core/src/main/java/org/eclipse/tycho/core/locking/FileLockServiceImpl.java

@@ -13,6 +13,7 @@
 
 package org.eclipse.tycho.core.locking;
 
+import java.io.Closeable;
 import java.io.File;
 import java.io.IOException;
 import java.util.Map;
@@ -27,6 +28,12 @@ public class FileLockServiceImpl implements FileLockService {
     private final Map<String, FileLockerImpl> lockers = new ConcurrentHashMap<>();
 
     @Override
+    public Closeable lock(File file, long timeout) {
+        FileLockerImpl locker = getFileLocker(file);
+        locker.lock(timeout);
+        return locker::release;
+    }
+
     public FileLockerImpl getFileLocker(File file) {
         String key;
         try {

tycho-core/src/main/java/org/eclipse/tycho/core/locking/FileLockerImpl.java

@@ -21,10 +21,9 @@
 import java.nio.file.Path;
 import java.nio.file.StandardOpenOption;
 
-import org.eclipse.tycho.FileLocker;
 import org.eclipse.tycho.LockTimeoutException;
 
-public class FileLockerImpl implements FileLocker {
+public class FileLockerImpl {
 
     private static final String LOCKFILE_SUFFIX = ".tycholock";
 
@@ -57,12 +56,10 @@ public FileLockerImpl(File file) {
         }
     }
 
-    @Override
     public void lock() {
         lock(10000L);
     }
 
-    @Override
     public void lock(long timeout) {
         if (timeout < 0) {
             throw new IllegalArgumentException("timeout must not be negative");
@@ -109,7 +106,6 @@ private FileLock aquireLock(long timeout) {
                 + lockMarkerFile.getAbsolutePath() + " for " + timeout + " msec");
     }
 
-    @Override
     public synchronized void release() {
         if (lock != null) {
             try {

tycho-core/src/main/java/org/eclipse/tycho/core/osgitools/DefaultBundleReader.java

@@ -36,7 +36,6 @@
 import org.codehaus.plexus.component.annotations.Requirement;
 import org.codehaus.plexus.logging.AbstractLogEnabled;
 import org.eclipse.tycho.FileLockService;
-import org.eclipse.tycho.FileLocker;
 import org.eclipse.tycho.TychoConstants;
 
 @Component(role = BundleReader.class)
@@ -189,9 +188,7 @@ public File getEntry(File bundleLocation, String path) {
                 throw new RuntimeException("can't get canonical path for " + cacheFile, e);
             }
             result = extractedFiles.computeIfAbsent(cacheKey, nil -> {
-                FileLocker locker = fileLockService.getFileLocker(outputDirectory);
-                locker.lock(LOCK_TIMEOUT);
-                try {
+                try (var locking = fileLockService.lock(outputDirectory, LOCK_TIMEOUT)) {
                     extractZipEntries(bundleLocation, path, outputDirectory);
                     if (cacheFile.exists()) {
                         return Optional.of(cacheFile);
@@ -200,8 +197,6 @@ public File getEntry(File bundleLocation, String path) {
                 } catch (IOException e) {
                     throw new RuntimeException(
                             "Can't extract '" + path + "' from " + bundleLocation + " to " + outputDirectory, e);
-                } finally {
-                    locker.release();
                 }
             });
         }

tycho-core/src/main/java/org/eclipse/tycho/p2/repository/FileBasedTychoRepositoryIndex.java

@@ -30,7 +30,6 @@
 import java.util.Set;
 
 import org.eclipse.tycho.FileLockService;
-import org.eclipse.tycho.FileLocker;
 import org.eclipse.tycho.core.shared.MavenContext;
 import org.eclipse.tycho.core.shared.MavenLogger;
 
@@ -47,7 +46,7 @@ public class FileBasedTychoRepositoryIndex implements TychoRepositoryIndex {
 
     private final File indexFile;
     private final MavenLogger logger;
-    private FileLocker fileLocker;
+    private final FileLockService fileLockService;
 
     private Set<GAV> addedGavs = new HashSet<>();
     private Set<GAV> removedGavs = new HashSet<>();
@@ -58,28 +57,17 @@ private FileBasedTychoRepositoryIndex(File indexFile, FileLockService fileLockSe
         super();
         this.indexFile = indexFile;
         this.mavenContext = mavenContext;
-        this.fileLocker = fileLockService.getFileLocker(indexFile);
+        this.fileLockService = fileLockService;
         this.logger = mavenContext.getLogger();
         if (indexFile.isFile()) {
-            lock();
-            try {
+            try (var locking = fileLockService.lock(indexFile)) {
                 gavs = read(new FileInputStream(indexFile));
             } catch (IOException e) {
                 throw new RuntimeException(e);
-            } finally {
-                unlock();
             }
         }
     }
 
-    private void lock() {
-        fileLocker.lock();
-    }
-
-    private void unlock() {
-        fileLocker.release();
-    }
-
     @Override
     public MavenContext getMavenContext() {
         return mavenContext;
@@ -118,8 +106,7 @@ public synchronized void save() throws IOException {
         if (!parentDir.isDirectory()) {
             parentDir.mkdirs();
         }
-        lock();
-        try {
+        try (var locking = fileLockService.lock(indexFile)) {
             reconcile();
             // minimize time window for corrupting the file by first writing to a temp file, then moving it
             File tempFile = File.createTempFile("index", "tmp", indexFile.getParentFile());
@@ -128,8 +115,6 @@ public synchronized void save() throws IOException {
                 indexFile.delete();
             }
             tempFile.renameTo(indexFile);
-        } finally {
-            unlock();
         }
     }
 

tycho-core/src/test/java/org/eclipse/tycho/core/locking/FileLockServiceTest.java

@@ -22,7 +22,6 @@
 import java.io.IOException;
 import java.util.Random;
 
-import org.eclipse.tycho.FileLocker;
 import org.eclipse.tycho.LockTimeoutException;
 import org.junit.Before;
 import org.junit.Rule;
@@ -42,90 +41,63 @@ public void setup() {
 
     @Test
     public void testIsLocked() throws IOException {
-        FileLockerImpl fileLocker = subject.getFileLocker(newTestFile());
-        fileLocker.lock();
-        try {
+        File file = newTestFile();
+        FileLockerImpl fileLocker = subject.getFileLocker(file);
+        try (var locking = subject.lock(file)) {
             assertTrue(fileLocker.isLocked());
-        } finally {
-            fileLocker.release();
-            assertFalse(fileLocker.isLocked());
         }
+        assertFalse(fileLocker.isLocked());
     }
 
     @Test(expected = IllegalArgumentException.class)
     public void testNegativeTimeout() throws IOException {
-        FileLocker fileLocker = subject.getFileLocker(newTestFile());
-        fileLocker.lock(-1L);
+        subject.lock(newTestFile(), -1L);
     }
 
     @Test
     public void testLockDirectory() throws IOException {
         File testDir = tempFolder.newFolder("test");
         FileLockerImpl fileLocker = subject.getFileLocker(testDir);
-        fileLocker.lock();
-        try {
+        try (var locking = subject.lock(testDir)) {
             assertTrue(fileLocker.isLocked());
             assertEquals(new File(testDir, ".tycholock").getCanonicalPath(),
                     fileLocker.lockMarkerFile.getCanonicalPath());
-        } finally {
-            fileLocker.release();
-        }
-    }
-
-    @Test
-    public void testLockReentranceSameLocker() throws IOException {
-        FileLocker fileLocker = subject.getFileLocker(newTestFile());
-        fileLocker.lock();
-        try {
-            // locks are not re-entrant
-            fileLocker.lock(0L);
-            fail("lock already held by same VM but could be acquired a second time");
-        } catch (LockTimeoutException e) {
-            // expected
-        } finally {
-            fileLocker.release();
         }
     }
 
     @Test
     public void testReuseLockerObject() throws IOException {
-        FileLockerImpl fileLocker = subject.getFileLocker(newTestFile());
-        lockAndRelease(fileLocker);
-        lockAndRelease(fileLocker);
+        File file = newTestFile();
+        FileLockerImpl fileLocker = subject.getFileLocker(file);
+        lockAndRelease(file, fileLocker);
+        lockAndRelease(file, fileLocker);
     }
 
-    private void lockAndRelease(FileLockerImpl fileLocker) {
+    private void lockAndRelease(File file, FileLockerImpl fileLocker) throws IOException {
         assertFalse(fileLocker.isLocked());
-        fileLocker.lock();
-        assertTrue(fileLocker.isLocked());
-        fileLocker.release();
+        try (var locking = subject.lock(file)) {
+            assertTrue(fileLocker.isLocked());
+        }
         assertFalse(fileLocker.isLocked());
     }
 
     @Test
     public void testLockReentranceDifferentLocker() throws IOException {
         final File testFile = newTestFile();
-        FileLocker fileLocker1 = subject.getFileLocker(testFile);
-        FileLocker fileLocker2 = subject.getFileLocker(testFile);
-        fileLocker1.lock();
-        try {
-            fileLocker2.lock(0L);
+        try (var locking = subject.lock(testFile)) {
+            subject.lock(testFile, 0L);
             fail("lock already held by same VM but could be acquired a second time");
         } catch (LockTimeoutException e) {
             // expected
-        } finally {
-            fileLocker1.release();
         }
     }
 
     @Test
     public void testLockedByOtherProcess() throws Exception {
         File testFile = newTestFile();
-        FileLocker locker = subject.getFileLocker(testFile);
         LockProcess lockProcess = new LockProcess(testFile, 200L);
         lockProcess.lockFileInForkedProcess();
-        try {
-            locker.lock(0L);
+        try (var locking = subject.lock(testFile)) {
             fail("lock already held by other VM but could be acquired a second time");
         } catch (LockTimeoutException e) {
             // expected
@@ -136,18 +108,15 @@ public void testLockedByOtherProcess() throws Exception {
     @Test
     public void testTimeout() throws Exception {
         File testFile = newTestFile();
-        FileLocker locker = subject.getFileLocker(testFile);
         long waitTime = 1000L;
         LockProcess lockProcess = new LockProcess(testFile, waitTime);
         long start = System.currentTimeMillis();
         lockProcess.lockFileInForkedProcess();
-        locker.lock(20000L);
-        try {
+        try (var locking = subject.lock(testFile, 20000L)) {
             long duration = System.currentTimeMillis() - start;
             assertTrue(duration >= waitTime);
         } finally {
             lockProcess.cleanup();
-            locker.release();
         }
     }
 
@@ -161,24 +130,21 @@ public void testRelease() throws Exception {
 
     @Test
     public void testMarkerFileDeletion() throws Exception {
-        FileLockerImpl locker = subject.getFileLocker(newTestFile());
-        locker.lock();
-        assertTrue(locker.lockMarkerFile.isFile());
-        locker.release();
+        File file = newTestFile();
+        FileLockerImpl locker = subject.getFileLocker(file);
+        try (var locking = subject.lock(file)) {
+            assertTrue(locker.lockMarkerFile.isFile());
+        }
         assertFalse(locker.lockMarkerFile.isFile());
     }
 
     @Test
     public void testURLEncoding() throws IOException {
         File testFile = new File(tempFolder.getRoot(), "file with spaces" + new Random().nextInt());
         File markerFile = new File(testFile.getAbsolutePath() + ".tycholock");
-        FileLocker fileLocker = subject.getFileLocker(testFile);
         assertFalse(markerFile.isFile());
-        fileLocker.lock();
-        try {
+        try (var locking = subject.lock(testFile)) {
             assertTrue(markerFile.isFile());
-        } finally {
-            fileLocker.release();
         }
     }