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

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

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2011 SAP AG and others.
+ * Copyright (c) 2011, 2023 SAP AG and others.
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
  * which accompanies this distribution, and is available at
@@ -13,6 +13,7 @@
 
 package org.eclipse.tycho;
 
+import java.io.Closeable;
 import java.io.File;
 
 /**
@@ -21,10 +22,19 @@
 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.
+     * <p>
+     * This is equivalent to {@link #lock(File, long)} with a timeout argument of 10 seconds.
+     * </p>
      */
-    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

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2011, 2020 SAP AG and others.
+ * Copyright (c) 2011, 2023 SAP AG and others.
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
  * which accompanies this distribution, and is available at
@@ -13,8 +13,10 @@
 
 package org.eclipse.tycho.core.locking;
 
+import java.io.Closeable;
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Path;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
@@ -24,17 +26,23 @@
 @Component(role = FileLockService.class)
 public class FileLockServiceImpl implements FileLockService {
 
-    private final Map<String, FileLockerImpl> lockers = new ConcurrentHashMap<>();
+    private final Map<Path, FileLockerImpl> lockers = new ConcurrentHashMap<>();
 
     @Override
-    public FileLockerImpl getFileLocker(File file) {
-        String key;
+    public Closeable lock(File file, long timeout) {
+        FileLockerImpl locker = getFileLocker(file.toPath());
+        locker.lock(timeout);
+        return locker::release;
+    }
+
+    FileLockerImpl getFileLocker(Path file) {
+        Path key;
         try {
-            key = file.getCanonicalPath();
+            key = file.toRealPath();
         } catch (IOException e) {
-            key = file.getAbsolutePath();
+            key = file.toAbsolutePath().normalize();
         }
-        return lockers.computeIfAbsent(key, k -> new FileLockerImpl(file));
+        return lockers.computeIfAbsent(key, FileLockerImpl::new);
     }
 
 }

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

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2011 SAP AG and others.
+ * Copyright (c) 2011, 2023 SAP AG and others.
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
  * which accompanies this distribution, and is available at
@@ -15,60 +15,46 @@
 
 import java.io.File;
 import java.io.IOException;
-import java.net.MalformedURLException;
 import java.nio.channels.FileChannel;
 import java.nio.channels.FileLock;
+import java.nio.file.Files;
 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";
 
-    final File lockMarkerFile;
+    final Path lockMarkerFile;
 
     private FileLock lock;
 
-    private File file;
+    private Path file;
 
-    public FileLockerImpl(File file) {
-        this.file = file;
+    FileLockerImpl(Path file) {
+        this.file = file.toAbsolutePath().normalize();
+        this.lockMarkerFile = Files.isDirectory(this.file) //
+                ? this.file.resolve(LOCKFILE_SUFFIX)
+                : this.file.getParent().resolve(this.file.getFileName() + LOCKFILE_SUFFIX);
         try {
-            if (file.isDirectory()) {
-                this.lockMarkerFile = new File(file, LOCKFILE_SUFFIX).getCanonicalFile();
-            } else {
-                this.lockMarkerFile = new File(file.getParentFile(), file.getName() + LOCKFILE_SUFFIX)
-                        .getCanonicalFile();
+            if (Files.isDirectory(lockMarkerFile)) {
+                throw new IllegalStateException(
+                        "Lock marker file " + lockMarkerFile + " already exists and is a directory");
             }
-            if (lockMarkerFile.isDirectory()) {
-                throw new RuntimeException("Lock marker file " + lockMarkerFile + " already exists and is a directory");
-            }
-            File parentDir = lockMarkerFile.getParentFile();
-            if (!parentDir.mkdirs() && !parentDir.isDirectory()) {
-                throw new RuntimeException("Could not create parent directory " + parentDir + " of lock marker file");
-            }
-        } catch (MalformedURLException e) {
-            throw new RuntimeException(e);
+            Files.createDirectories(lockMarkerFile.getParent());
         } catch (IOException e) {
-            throw new RuntimeException(e);
+            throw new IllegalStateException(e);
         }
     }
 
-    @Override
-    public void lock() {
-        lock(10000L);
-    }
-
-    @Override
-    public void lock(long timeout) {
+    void lock(long timeout) {
         if (timeout < 0) {
             throw new IllegalArgumentException("timeout must not be negative");
         }
         if (lock != null) {
-            throw new LockTimeoutException("already locked file " + file.getAbsolutePath());
+            throw new LockTimeoutException("already locked file " + file);
         }
         lock = aquireLock(timeout);
 
@@ -81,8 +67,7 @@ private FileLock aquireLock(long timeout) {
         for (long i = 0; i < maxTries; i++) {
             try {
                 if (channel == null) {
-                    Path path = lockMarkerFile.toPath();
-                    channel = FileChannel.open(path, StandardOpenOption.WRITE, StandardOpenOption.CREATE);
+                    channel = FileChannel.open(lockMarkerFile, StandardOpenOption.WRITE, StandardOpenOption.CREATE);
                 }
                 FileLock fileLock = channel.tryLock();
                 if (fileLock != null) {
@@ -105,26 +90,22 @@ private FileLock aquireLock(long timeout) {
                 channel = null;
             }
         }
-        throw new LockTimeoutException("lock timeout: Could not acquire lock on file "
-                + lockMarkerFile.getAbsolutePath() + " for " + timeout + " msec");
+        throw new LockTimeoutException(
+                "lock timeout: Could not acquire lock on file " + lockMarkerFile + " for " + timeout + " msec");
     }
 
-    @Override
-    public synchronized void release() {
+    synchronized void release() {
         if (lock != null) {
             try {
                 lock.acquiredBy().close();
             } catch (Exception e) {
             }
             lock = null;
-            if (!lockMarkerFile.delete()) {
-                lockMarkerFile.deleteOnExit();
+            File lockFile = lockMarkerFile.toFile();
+            if (!lockFile.delete()) {
+                lockFile.deleteOnExit();
             }
         }
     }
 
-    public synchronized boolean isLocked() {
-        return lock != null;
-    }
-
 }

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 locked = 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 locked = 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 locked = 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();
         }
     }