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 @@
maven-resolver-test-util
test
+
+ com.google.jimfs
+ jimfs
+ test
+
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.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 Properties read(File file) {
@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 Properties update(File file, Map updates) {
@Override
public Properties update(Path path, Map 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 update : updates.entrySet()) {
- if (update.getValue() == null) {
- props.remove(update.getKey());
- } else {
- props.setProperty(update.getKey(), update.getValue());
- }
+ for (Map.Entry 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.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 void touchArtifact(RepositorySystemSession session, UpdateCheck 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 @@ void testRead() throws Exception {
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 updates = new HashMap<>();
updates.put("key1", "v");
@@ -87,36 +142,50 @@ void testUpdate() throws Exception {
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 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 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 {