From 96d7bbc6b85812fd035af8bd528faccf11a6d329 Mon Sep 17 00:00:00 2001 From: Hannes Wellmann Date: Mon, 25 Sep 2023 00:22:40 +0200 Subject: [PATCH] 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 } --- .../org/eclipse/tycho/FileLockService.java | 15 +++- .../java/org/eclipse/tycho/FileLocker.java | 44 ---------- .../core/locking/FileLockServiceImpl.java | 7 ++ .../tycho/core/locking/FileLockerImpl.java | 6 +- .../core/osgitools/DefaultBundleReader.java | 7 +- .../FileBasedTychoRepositoryIndex.java | 23 +----- .../core/locking/FileLockServiceTest.java | 82 ++++++------------- .../LocalMavenRepositoryTool.java | 13 +-- .../p2/publisher/PublishProductMojo.java | 15 ++-- .../packaging/FeatureXmlTransformer.java | 28 +++---- .../tycho/packaging/UpdateSiteAssembler.java | 11 +-- .../tycho/test/util/NoopFileLockService.java | 19 +---- 12 files changed, 73 insertions(+), 197 deletions(-) delete mode 100644 tycho-api/src/main/java/org/eclipse/tycho/FileLocker.java diff --git a/tycho-api/src/main/java/org/eclipse/tycho/FileLockService.java b/tycho-api/src/main/java/org/eclipse/tycho/FileLockService.java index 21e525627a..a841ce47cf 100644 --- a/tycho-api/src/main/java/org/eclipse/tycho/FileLockService.java +++ b/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); } diff --git a/tycho-api/src/main/java/org/eclipse/tycho/FileLocker.java b/tycho-api/src/main/java/org/eclipse/tycho/FileLocker.java deleted file mode 100644 index bf3b7296e4..0000000000 --- a/tycho-api/src/main/java/org/eclipse/tycho/FileLocker.java +++ /dev/null @@ -1,44 +0,0 @@ -/******************************************************************************* - * Copyright (c) 2011 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 - * https://www.eclipse.org/legal/epl-2.0/ - * - * SPDX-License-Identifier: EPL-2.0 - * - * Contributors: - * SAP AG - initial API and implementation - *******************************************************************************/ - -package org.eclipse.tycho; - -/** - * Provides process-level file locking. Locking is advisory, meaning that processes must - * cooperatively use the same locking mechanism. - */ -public interface FileLocker { - - /** - * Equivalent to {{@link #lock(long)} with a timeout argument of 10000 milliseconds. - */ - public void lock() throws LockTimeoutException; - - /** - * Attempt to lock the file associated with this locker object. Note that technically, not the - * file itself is locked, but an empty marker file next to it. - * - * @param timeout - * timeout in milliseconds - * - * @throws LockTimeoutException - * if lock cannot be obtained for more than the specified timeout in millseconds. - */ - public void lock(long timeout) throws LockTimeoutException; - - /** - * Release the lock if acquired. Also removes the lock marker file. - */ - public void release(); - -} diff --git a/tycho-core/src/main/java/org/eclipse/tycho/core/locking/FileLockServiceImpl.java b/tycho-core/src/main/java/org/eclipse/tycho/core/locking/FileLockServiceImpl.java index 270afe2d9f..4158a84465 100644 --- a/tycho-core/src/main/java/org/eclipse/tycho/core/locking/FileLockServiceImpl.java +++ b/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 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 { diff --git a/tycho-core/src/main/java/org/eclipse/tycho/core/locking/FileLockerImpl.java b/tycho-core/src/main/java/org/eclipse/tycho/core/locking/FileLockerImpl.java index 7fcb2a07ff..0dcdd4daa4 100644 --- a/tycho-core/src/main/java/org/eclipse/tycho/core/locking/FileLockerImpl.java +++ b/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 { diff --git a/tycho-core/src/main/java/org/eclipse/tycho/core/osgitools/DefaultBundleReader.java b/tycho-core/src/main/java/org/eclipse/tycho/core/osgitools/DefaultBundleReader.java index 841247215c..09dcba25d7 100644 --- a/tycho-core/src/main/java/org/eclipse/tycho/core/osgitools/DefaultBundleReader.java +++ b/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(); } }); } diff --git a/tycho-core/src/main/java/org/eclipse/tycho/p2/repository/FileBasedTychoRepositoryIndex.java b/tycho-core/src/main/java/org/eclipse/tycho/p2/repository/FileBasedTychoRepositoryIndex.java index a4f3814b39..7ac287de8d 100644 --- a/tycho-core/src/main/java/org/eclipse/tycho/p2/repository/FileBasedTychoRepositoryIndex.java +++ b/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 addedGavs = new HashSet<>(); private Set 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(); } } diff --git a/tycho-core/src/test/java/org/eclipse/tycho/core/locking/FileLockServiceTest.java b/tycho-core/src/test/java/org/eclipse/tycho/core/locking/FileLockServiceTest.java index 67d8672c32..17aa04eefc 100644 --- a/tycho-core/src/test/java/org/eclipse/tycho/core/locking/FileLockServiceTest.java +++ b/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,10 +130,11 @@ 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()); } @@ -172,13 +142,9 @@ public void testMarkerFileDeletion() throws Exception { 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(); } } diff --git a/tycho-its/src/test/java/org/eclipse/tycho/test/pomDependencyConsider/LocalMavenRepositoryTool.java b/tycho-its/src/test/java/org/eclipse/tycho/test/pomDependencyConsider/LocalMavenRepositoryTool.java index 5e1be22158..e7c5d6ef6c 100644 --- a/tycho-its/src/test/java/org/eclipse/tycho/test/pomDependencyConsider/LocalMavenRepositoryTool.java +++ b/tycho-its/src/test/java/org/eclipse/tycho/test/pomDependencyConsider/LocalMavenRepositoryTool.java @@ -27,7 +27,6 @@ import java.util.Set; import org.eclipse.tycho.FileLockService; -import org.eclipse.tycho.FileLocker; import org.eclipse.tycho.test.util.EnvironmentUtil; public class LocalMavenRepositoryTool { @@ -59,12 +58,8 @@ public File getArtifactFile(String groupId, String artifactId, String version, S public Set getArtifactIndexLines() throws IOException { File indexFile = getArtifactIndexFile(); - FileLocker locker = fileLockService.getFileLocker(indexFile); - locker.lock(); - try { + try (var locking = fileLockService.lock(indexFile)) { return readLines(indexFile); - } finally { - locker.release(); } } @@ -95,14 +90,10 @@ public void removeLinesFromMetadataIndex(String... linesToBeRemoved) throws IOEx private void filterLinesFromIndex(File indexFile, Set toBeRemoved) throws FileNotFoundException, IOException { - FileLocker locker = fileLockService.getFileLocker(indexFile); - locker.lock(); - try { + try (var locking = fileLockService.lock(indexFile)) { Set currentLines = readLines(indexFile); currentLines.removeAll(toBeRemoved); writeLines(indexFile, currentLines); - } finally { - locker.release(); } } diff --git a/tycho-p2-publisher-plugin/src/main/java/org/eclipse/tycho/plugins/p2/publisher/PublishProductMojo.java b/tycho-p2-publisher-plugin/src/main/java/org/eclipse/tycho/plugins/p2/publisher/PublishProductMojo.java index 38802be410..1b26620eb6 100644 --- a/tycho-p2-publisher-plugin/src/main/java/org/eclipse/tycho/plugins/p2/publisher/PublishProductMojo.java +++ b/tycho-p2-publisher-plugin/src/main/java/org/eclipse/tycho/plugins/p2/publisher/PublishProductMojo.java @@ -21,14 +21,17 @@ import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugin.MojoFailureException; -import org.apache.maven.plugins.annotations.*; +import org.apache.maven.plugins.annotations.Component; +import org.apache.maven.plugins.annotations.LifecyclePhase; +import org.apache.maven.plugins.annotations.Mojo; +import org.apache.maven.plugins.annotations.Parameter; +import org.apache.maven.plugins.annotations.ResolutionScope; import org.codehaus.plexus.archiver.ArchiverException; import org.codehaus.plexus.archiver.UnArchiver; import org.eclipse.tycho.ArtifactDescriptor; import org.eclipse.tycho.ArtifactType; import org.eclipse.tycho.DependencyArtifacts; import org.eclipse.tycho.FileLockService; -import org.eclipse.tycho.FileLocker; import org.eclipse.tycho.Interpolator; import org.eclipse.tycho.PackagingType; import org.eclipse.tycho.PlatformPropertiesUtils; @@ -129,19 +132,15 @@ private File getExpandedLauncherBinaries() throws MojoExecutionException, MojoFa return unzipped.getAbsoluteFile(); } try { - FileLocker locker = fileLockService.getFileLocker(equinoxExecFeature); - locker.lock(); - try { + try (var locking = fileLockService.lock(equinoxExecFeature)) { // unzip now then: unzipped.mkdirs(); deflater.setSourceFile(equinoxExecFeature); deflater.setDestDirectory(unzipped); deflater.extract(); return unzipped.getAbsoluteFile(); - } finally { - locker.release(); } - } catch (ArchiverException e) { + } catch (ArchiverException | IOException e) { throw new MojoFailureException("Unable to unzip the equinox executable feature", e); } } diff --git a/tycho-packaging-plugin/src/main/java/org/eclipse/tycho/packaging/FeatureXmlTransformer.java b/tycho-packaging-plugin/src/main/java/org/eclipse/tycho/packaging/FeatureXmlTransformer.java index 7dd4099b9d..ac25d067a2 100644 --- a/tycho-packaging-plugin/src/main/java/org/eclipse/tycho/packaging/FeatureXmlTransformer.java +++ b/tycho-packaging-plugin/src/main/java/org/eclipse/tycho/packaging/FeatureXmlTransformer.java @@ -32,7 +32,6 @@ import org.eclipse.tycho.ArtifactKey; import org.eclipse.tycho.ArtifactType; import org.eclipse.tycho.FileLockService; -import org.eclipse.tycho.FileLocker; import org.eclipse.tycho.IllegalArtifactReferenceException; import org.eclipse.tycho.TargetPlatform; import org.eclipse.tycho.model.Feature; @@ -174,25 +173,18 @@ private void setDownloadAndInstallSize(PluginRef pluginRefToEdit, File artifact) protected long getInstallSize(File location) { long installSize = 0; - FileLocker locker = fileLockService.getFileLocker(location); - locker.lock(); - try { - try { - try (JarFile jar = new JarFile(location)) { - Enumeration entries = jar.entries(); - while (entries.hasMoreElements()) { - JarEntry entry = entries.nextElement(); - long entrySize = entry.getSize(); - if (entrySize > 0) { - installSize += entrySize; - } - } + try (var locking = fileLockService.lock(location); // + JarFile jar = new JarFile(location);) { + Enumeration entries = jar.entries(); + while (entries.hasMoreElements()) { + JarEntry entry = entries.nextElement(); + long entrySize = entry.getSize(); + if (entrySize > 0) { + installSize += entrySize; } - } catch (IOException e) { - throw new RuntimeException("Could not determine installation size of file " + location, e); } - } finally { - locker.release(); + } catch (IOException e) { + throw new RuntimeException("Could not determine installation size of file " + location, e); } return installSize; } diff --git a/tycho-packaging-plugin/src/main/java/org/eclipse/tycho/packaging/UpdateSiteAssembler.java b/tycho-packaging-plugin/src/main/java/org/eclipse/tycho/packaging/UpdateSiteAssembler.java index a275afbeaf..8e78370ef8 100644 --- a/tycho-packaging-plugin/src/main/java/org/eclipse/tycho/packaging/UpdateSiteAssembler.java +++ b/tycho-packaging-plugin/src/main/java/org/eclipse/tycho/packaging/UpdateSiteAssembler.java @@ -28,7 +28,6 @@ import org.codehaus.plexus.component.repository.exception.ComponentLookupException; import org.codehaus.plexus.util.FileUtils; import org.eclipse.tycho.FileLockService; -import org.eclipse.tycho.FileLocker; import org.eclipse.tycho.ReactorProject; import org.eclipse.tycho.core.ArtifactDependencyVisitor; import org.eclipse.tycho.core.FeatureDescription; @@ -199,15 +198,11 @@ private void unpackJar(File location, File outputJar) { unzip.setSourceFile(location); unzip.setDestDirectory(outputJar); - FileLocker locker = fileLockService.getFileLocker(location); - locker.lock(); - try { + try (var locking = fileLockService.lock(location)) { unzip.extract(); - } catch (ArchiverException e) { + } catch (ArchiverException | IOException e) { throw new RuntimeException("Could not unpack jar", e); - } finally { - locker.release(); - } + } } private void copyDir(File location, File outputJar) { diff --git a/tycho-testing-harness/src/main/java/org/eclipse/tycho/test/util/NoopFileLockService.java b/tycho-testing-harness/src/main/java/org/eclipse/tycho/test/util/NoopFileLockService.java index 51751e8ade..18a8f9fdc2 100644 --- a/tycho-testing-harness/src/main/java/org/eclipse/tycho/test/util/NoopFileLockService.java +++ b/tycho-testing-harness/src/main/java/org/eclipse/tycho/test/util/NoopFileLockService.java @@ -13,29 +13,16 @@ package org.eclipse.tycho.test.util; +import java.io.Closeable; import java.io.File; import org.eclipse.tycho.FileLockService; -import org.eclipse.tycho.FileLocker; -import org.eclipse.tycho.LockTimeoutException; public class NoopFileLockService implements FileLockService { @Override - public FileLocker getFileLocker(File file) { - return new FileLocker() { - - @Override - public void release() { - } - - @Override - public void lock() { - } - - @Override - public void lock(long timeout) throws LockTimeoutException { - } + public Closeable lock(File file, long timeout) { + return () -> { }; }