This is an automated email from the ASF dual-hosted git repository.
cstamas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-resolver.git
The following commit(s) were added to refs/heads/master by this push:
new 5bf4b92a6 TrackingFileManager changes (#1692)
5bf4b92a6 is described below
commit 5bf4b92a6b04c3d1a836a90378b97e04aec99462
Author: Tamas Cservenak <[email protected]>
AuthorDate: Fri Nov 28 15:44:27 2025 +0100
TrackingFileManager changes (#1692)
Drop out of band deletion, let TFM delete as well. Also, simplify but do
not drop locking, is needed for interoperability with 3.9.11 and 4.0.0-rc-5 and
older released versions. Make FNFEx ignored in case of read/delete.
Backport to 1.9.x is here
https://github.com/apache/maven-resolver/pull/1695.
---
maven-resolver-impl/pom.xml | 5 +
.../internal/impl/DefaultTrackingFileManager.java | 109 +++++++++---------
.../internal/impl/DefaultUpdateCheckManager.java | 8 +-
.../aether/internal/impl/TrackingFileManager.java | 16 +++
.../impl/DefaultTrackingFileManagerTest.java | 123 ++++++++++++++++-----
5 files changed, 176 insertions(+), 85 deletions(-)
diff --git a/maven-resolver-impl/pom.xml b/maven-resolver-impl/pom.xml
index 6a0cfd406..60950c0d2 100644
--- a/maven-resolver-impl/pom.xml
+++ b/maven-resolver-impl/pom.xml
@@ -96,6 +96,11 @@
<artifactId>maven-resolver-test-util</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>com.google.jimfs</groupId>
+ <artifactId>jimfs</artifactId>
+ <scope>test</scope>
+ </dependency>
</dependencies>
<build>
diff --git
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultTrackingFileManager.java
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultTrackingFileManager.java
index 53f02d9ab..28b3b0fc9 100644
---
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultTrackingFileManager.java
+++
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultTrackingFileManager.java
@@ -31,6 +31,7 @@ import java.nio.channels.FileChannel;
import java.nio.channels.FileLock;
import java.nio.channels.OverlappingFileLockException;
import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.nio.file.StandardOpenOption;
import java.util.Map;
@@ -61,15 +62,14 @@ public final class DefaultTrackingFileManager implements
TrackingFileManager {
@Override
public Properties read(Path path) {
if (Files.isReadable(path)) {
- synchronized (getMutex(path)) {
- try {
- long fileSize = Files.size(path);
- try (FileChannel fileChannel = FileChannel.open(path,
StandardOpenOption.READ);
- FileLock unused = fileLock(fileChannel,
Math.max(1, fileSize), true)) {
- Properties props = new Properties();
- props.load(Channels.newInputStream(fileChannel));
- return props;
- }
+ synchronized (mutex(path)) {
+ try (FileChannel fileChannel = FileChannel.open(path,
StandardOpenOption.READ);
+ FileLock unused = fileLock(fileChannel, true)) {
+ Properties props = new Properties();
+ props.load(Channels.newInputStream(fileChannel));
+ return props;
+ } catch (NoSuchFileException e) {
+ LOGGER.debug("No such file to read {}: {}", path,
e.getMessage());
} catch (IOException e) {
LOGGER.warn("Failed to read tracking file '{}'", path, e);
throw new UncheckedIOException(e);
@@ -87,72 +87,79 @@ public final class DefaultTrackingFileManager implements
TrackingFileManager {
@Override
public Properties update(Path path, Map<String, String> updates) {
- Properties props = new Properties();
try {
Files.createDirectories(path.getParent());
} catch (IOException e) {
LOGGER.warn("Failed to create tracking file parent '{}'", path, e);
throw new UncheckedIOException(e);
}
-
- synchronized (getMutex(path)) {
- try {
- long fileSize;
- try {
- fileSize = Files.size(path);
- } catch (IOException e) {
- fileSize = 0L;
+ synchronized (mutex(path)) {
+ try (FileChannel fileChannel = FileChannel.open(
+ path, StandardOpenOption.READ,
StandardOpenOption.WRITE, StandardOpenOption.CREATE);
+ FileLock unused = fileLock(fileChannel, false)) {
+ Properties props = new Properties();
+ if (fileChannel.size() > 0) {
+ props.load(Channels.newInputStream(fileChannel));
}
- try (FileChannel fileChannel = FileChannel.open(
- path, StandardOpenOption.READ,
StandardOpenOption.WRITE, StandardOpenOption.CREATE);
- FileLock unused = fileLock(fileChannel, Math.max(1,
fileSize), false)) {
- if (fileSize > 0) {
- props.load(Channels.newInputStream(fileChannel));
- }
- for (Map.Entry<String, String> update :
updates.entrySet()) {
- if (update.getValue() == null) {
- props.remove(update.getKey());
- } else {
- props.setProperty(update.getKey(),
update.getValue());
- }
+ for (Map.Entry<String, String> update : updates.entrySet()) {
+ if (update.getValue() == null) {
+ props.remove(update.getKey());
+ } else {
+ props.setProperty(update.getKey(), update.getValue());
}
-
- LOGGER.debug("Writing tracking file '{}'", path);
- ByteArrayOutputStream stream = new
ByteArrayOutputStream(1024 * 2);
- props.store(
- stream,
- "NOTE: This is a Maven Resolver internal
implementation file"
- + ", its format can be changed without
prior notice.");
- fileChannel.position(0);
- int written =
fileChannel.write(ByteBuffer.wrap(stream.toByteArray()));
- fileChannel.truncate(written);
}
+
+ LOGGER.debug("Writing tracking file '{}'", path);
+ ByteArrayOutputStream stream = new ByteArrayOutputStream(1024
* 2);
+ props.store(
+ stream,
+ "NOTE: This is a Maven Resolver internal
implementation file"
+ + ", its format can be changed without prior
notice.");
+ fileChannel.position(0);
+ int written =
fileChannel.write(ByteBuffer.wrap(stream.toByteArray()));
+ fileChannel.truncate(written);
+ return props;
} catch (IOException e) {
LOGGER.warn("Failed to write tracking file '{}'", path, e);
throw new UncheckedIOException(e);
}
}
+ }
- return props;
+ @Override
+ public boolean delete(File file) {
+ return delete(file.toPath());
+ }
+
+ @Override
+ public boolean delete(Path path) {
+ if (Files.isReadable(path)) {
+ synchronized (mutex(path)) {
+ try (FileChannel fileChannel = FileChannel.open(path,
StandardOpenOption.WRITE);
+ FileLock unused = fileLock(fileChannel, false)) {
+ Files.delete(path);
+ return true;
+ } catch (NoSuchFileException e) {
+ LOGGER.debug("No such file to delete {}: {}", path,
e.getMessage());
+ } catch (IOException e) {
+ LOGGER.warn("Failed to delete tracking file '{}'", path,
e);
+ throw new UncheckedIOException(e);
+ }
+ }
+ }
+ return false;
}
- private Object getMutex(Path path) {
- // The interned string of path is (mis)used as mutex, to exclude
different threads going for same file,
- // as JVM file locking happens on JVM not on Thread level. This is how
original code did it ¯\_(ツ)_/¯
- /*
- * NOTE: Locks held by one JVM must not overlap and using the
canonical path is our best bet, still another
- * piece of code might have locked the same file (unlikely though) or
the canonical path fails to capture file
- * identity sufficiently as is the case with Java 1.6 and symlinks on
Windows.
- */
+ private Object mutex(Path path) {
return path.toAbsolutePath().normalize().toString().intern();
}
- private FileLock fileLock(FileChannel channel, long size, boolean shared)
throws IOException {
+ private FileLock fileLock(FileChannel channel, boolean shared) throws
IOException {
FileLock lock = null;
for (int attempts = 8; attempts >= 0; attempts--) {
try {
- lock = channel.lock(0, size, shared);
+ lock = channel.lock(0, Long.MAX_VALUE, shared);
break;
} catch (OverlappingFileLockException e) {
if (attempts <= 0) {
diff --git
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java
index 3d6133935..02e0da8f5 100644
---
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java
+++
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java
@@ -22,8 +22,6 @@ import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Singleton;
-import java.io.IOException;
-import java.io.UncheckedIOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Collections;
@@ -482,11 +480,7 @@ public class DefaultUpdateCheckManager implements
UpdateCheckManager {
Properties props = write(touchPath, dataKey, transferKey,
check.getException());
if (Files.exists(artifactPath) && !hasErrors(props)) {
- try {
- Files.deleteIfExists(touchPath);
- } catch (IOException e) {
- throw new UncheckedIOException(e);
- }
+ trackingFileManager.delete(touchPath);
}
}
diff --git
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/TrackingFileManager.java
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/TrackingFileManager.java
index d7a03ab87..886dafa71 100644
---
a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/TrackingFileManager.java
+++
b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/TrackingFileManager.java
@@ -54,4 +54,20 @@ public interface TrackingFileManager {
* @since 2.0.0
*/
Properties update(Path path, Map<String, String> updates);
+
+ /**
+ * Deletes the specified properties file, if exists. If file existed and
was deleted, returns {@code true}.
+ *
+ * @deprecated Use {@link #delete(Path)} instead.
+ * @since 1.9.25
+ */
+ @Deprecated
+ boolean delete(File file);
+
+ /**
+ * Deletes the specified properties file, if exists. If file existed and
was deleted, returns {@code true}.
+ *
+ * @since 2.0.14
+ */
+ boolean delete(Path path);
}
diff --git
a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultTrackingFileManagerTest.java
b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultTrackingFileManagerTest.java
index dfa5094db..c58606335 100644
---
a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultTrackingFileManagerTest.java
+++
b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultTrackingFileManagerTest.java
@@ -18,29 +18,83 @@
*/
package org.eclipse.aether.internal.impl;
-import java.io.File;
+import java.io.IOException;
import java.nio.charset.StandardCharsets;
+import java.nio.file.FileSystem;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.Properties;
-import org.eclipse.aether.internal.test.util.TestFileUtils;
-import org.junit.jupiter.api.Test;
+import com.google.common.jimfs.Configuration;
+import com.google.common.jimfs.Jimfs;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
import static org.junit.jupiter.api.Assertions.*;
-/**
- */
public class DefaultTrackingFileManagerTest {
+ public enum FS {
+ DEFAULT,
+ JIMFS_UNIX,
+ JIMFS_WINDOWS;
+
+ @Override
+ public String toString() {
+ return super.name().toLowerCase(Locale.ENGLISH);
+ }
+ }
+
+ private FileSystem fileSystem;
+
+ private Path createTmpFile(FS fs) throws IOException {
+ return createTmpFile(fs, 1);
+ }
+
+ private Path createTmpFile(FS fs, int iterations) throws IOException {
+ byte[] payload = "#COMMENT\nkey1=value1\nkey2 :
value2\n".getBytes(StandardCharsets.UTF_8);
+ if (iterations > 1) {
+ int payloadLength = payload.length;
+ byte[] newPayload = new byte[payloadLength * iterations];
+ for (int i = 0; i < iterations; i++) {
+ System.arraycopy(payload, 0, newPayload, i * payloadLength,
payloadLength);
+ }
+ payload = newPayload;
+ }
+ Path tempFile;
+ if (fs == FS.DEFAULT) {
+ tempFile = Files.createTempFile(getClass().getSimpleName(),
".tmp");
+ } else if (fs == FS.JIMFS_UNIX || fs == FS.JIMFS_WINDOWS) {
+ fileSystem = Jimfs.newFileSystem(fs == FS.JIMFS_WINDOWS ?
Configuration.windows() : Configuration.unix());
+ tempFile = fileSystem.getPath("tmp/file.properties");
+ } else {
+ throw new IllegalStateException("unsupported FS: " + fs);
+ }
+ Files.createDirectories(tempFile.getParent());
+ Files.write(tempFile, payload);
+ return tempFile;
+ }
- @Test
- void testRead() throws Exception {
+ @AfterEach
+ void cleanup() throws IOException {
+ if (fileSystem != null) {
+ fileSystem.close();
+ }
+ }
+
+ @ParameterizedTest
+ @EnumSource(FS.class)
+ void testRead(FS fs) throws Exception {
TrackingFileManager tfm = new DefaultTrackingFileManager();
- File propFile =
TestFileUtils.createTempFile("#COMMENT\nkey1=value1\nkey2 : value2");
+ Path propFile = createTmpFile(fs);
Properties props = tfm.read(propFile);
assertNotNull(props);
@@ -48,30 +102,31 @@ public class DefaultTrackingFileManagerTest {
assertEquals("value1", props.get("key1"));
assertEquals("value2", props.get("key2"));
- assertTrue(propFile.delete(), "Leaked file: " + propFile);
+ assertDoesNotThrow(() -> Files.delete(propFile), "Leaked file" +
propFile);
props = tfm.read(propFile);
assertNull(props, String.valueOf(props));
}
- @Test
- void testReadNoFileLeak() throws Exception {
+ @ParameterizedTest
+ @EnumSource(FS.class)
+ void testReadNoFileLeak(FS fs) throws Exception {
TrackingFileManager tfm = new DefaultTrackingFileManager();
for (int i = 0; i < 1000; i++) {
- File propFile =
TestFileUtils.createTempFile("#COMMENT\nkey1=value1\nkey2 : value2");
+ Path propFile = createTmpFile(fs);
assertNotNull(tfm.read(propFile));
- assertTrue(propFile.delete(), "Leaked file: " + propFile);
+ assertDoesNotThrow(() -> Files.delete(propFile), "Leaked file" +
propFile);
}
}
- @Test
- void testUpdate() throws Exception {
+ @ParameterizedTest
+ @EnumSource(FS.class)
+ void testUpdate(FS fs) throws Exception {
TrackingFileManager tfm = new DefaultTrackingFileManager();
// NOTE: The excessive repetitions are to check the update properly
truncates the file
- File propFile =
- TestFileUtils.createTempFile("key1=value1\nkey2 :
value2\n".getBytes(StandardCharsets.UTF_8), 1000);
+ Path propFile = createTmpFile(fs, 1000);
Map<String, String> updates = new HashMap<>();
updates.put("key1", "v");
@@ -87,36 +142,50 @@ public class DefaultTrackingFileManagerTest {
assertNull(props.get("key2"), String.valueOf(props.get("key2")));
}
- @Test
- void testUpdateNoFileLeak() throws Exception {
+ @ParameterizedTest
+ @EnumSource(FS.class)
+ void testUpdateNoFileLeak(FS fs) throws Exception {
TrackingFileManager tfm = new DefaultTrackingFileManager();
Map<String, String> updates = new HashMap<>();
updates.put("k", "v");
for (int i = 0; i < 1000; i++) {
- File propFile =
TestFileUtils.createTempFile("#COMMENT\nkey1=value1\nkey2 : value2");
+ Path propFile = createTmpFile(fs);
assertNotNull(tfm.update(propFile, updates));
- assertTrue(propFile.delete(), "Leaked file: " + propFile);
+ assertDoesNotThrow(() -> Files.delete(propFile), "Leaked file" +
propFile);
+ }
+ }
+
+ @ParameterizedTest
+ @EnumSource(FS.class)
+ public void testDeleteFileIsGone(FS fs) throws Exception {
+ TrackingFileManager tfm = new DefaultTrackingFileManager();
+
+ for (int i = 0; i < 1000; i++) {
+ Path propFile = createTmpFile(fs);
+ assertTrue(tfm.delete(propFile));
+ assertFalse(Files.exists(propFile), "File is not gone");
}
}
- @Test
- void testLockingOnCanonicalPath() throws Exception {
+ @ParameterizedTest
+ @EnumSource(FS.class)
+ void testLockingOnCanonicalPath(FS fs) throws Exception {
final TrackingFileManager tfm = new DefaultTrackingFileManager();
- final File propFile =
TestFileUtils.createTempFile("#COMMENT\nkey1=value1\nkey2 : value2");
+ Path propFile = createTmpFile(fs);
final List<Throwable> errors = Collections.synchronizedList(new
ArrayList<>());
Thread[] threads = new Thread[4];
for (int i = 0; i < threads.length; i++) {
- String path = propFile.getParent();
+ String path = propFile.getParent().toString();
for (int j = 0; j < i; j++) {
path += "/.";
}
- path += "/" + propFile.getName();
- final File file = new File(path);
+ path += "/" + propFile.getFileName();
+ final Path file = fileSystem != null ? fileSystem.getPath(path) :
Paths.get(path);
threads[i] = new Thread(() -> {
try {
