From cb50e5c35cbd6190b707600528338218a4176f5a Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 27 Nov 2025 19:49:09 +0100 Subject: [PATCH 1/6] [1.9.x] TrackingFileManager changes Backport from master. Needs Maven changes as well. --- .../impl/DefaultTrackingFileManager.java | 45 +++++++++++-------- .../impl/DefaultUpdateCheckManager.java | 2 +- .../internal/impl/TrackingFileManager.java | 7 +++ 3 files changed, 35 insertions(+), 19 deletions(-) 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 8f41f7d2f..2fc03bfde 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 @@ -32,6 +32,7 @@ import java.nio.channels.FileLock; import java.nio.channels.OverlappingFileLockException; import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import java.util.Map; import java.util.Properties; @@ -54,12 +55,14 @@ public final class DefaultTrackingFileManager implements TrackingFileManager { @Override public Properties read(File file) { if (Files.isReadable(file.toPath())) { - synchronized (getMutex(file)) { + synchronized (mutex(file)) { try (FileInputStream stream = new FileInputStream(file); FileLock unused = fileLock(stream.getChannel(), Math.max(1, file.length()), true)) { Properties props = new Properties(); props.load(stream); return props; + } catch (NoSuchFileException e) { + LOGGER.debug("No such file to read {}: {}", file, e.getMessage()); } catch (IOException e) { LOGGER.warn("Failed to read tracking file '{}'", file, e); throw new UncheckedIOException(e); @@ -71,16 +74,14 @@ public Properties read(File file) { @Override public Properties update(File file, Map updates) { - Properties props = new Properties(); - try { Files.createDirectories(file.getParentFile().toPath()); } catch (IOException e) { LOGGER.warn("Failed to create tracking file parent '{}'", file, e); throw new UncheckedIOException(e); } - - synchronized (getMutex(file)) { + Properties props = new Properties(); + synchronized (mutex(file)) { try (RandomAccessFile raf = new RandomAccessFile(file, "rw"); FileLock unused = fileLock(raf.getChannel(), Math.max(1, raf.length()), false)) { if (raf.length() > 0) { @@ -115,21 +116,29 @@ public Properties update(File file, Map updates) { return props; } - private Object getMutex(File file) { + @Override + public boolean delete(File file) { + if (Files.isReadable(file.toPath())) { + synchronized (mutex(file)) { + try (RandomAccessFile raf = new RandomAccessFile(file, "rw"); + FileLock unused = fileLock(raf.getChannel(), Math.max(1, raf.length()), false)) { + Files.delete(file.toPath()); + return true; + } catch (NoSuchFileException e) { + LOGGER.debug("No such file to delete {}: {}", file, e.getMessage()); + } catch (IOException e) { + LOGGER.warn("Failed to delete tracking file '{}'", file, e); + throw new UncheckedIOException(e); + } + } + } + return false; + } + + private Object mutex(File file) { // 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. - */ - try { - return file.getCanonicalPath().intern(); - } catch (IOException e) { - LOGGER.warn("Failed to canonicalize path {}", file, e); - // TODO This is code smell and deprecated - return file.getAbsolutePath().intern(); - } + return file.toPath().toAbsolutePath().normalize().toString().intern(); } private FileLock fileLock(FileChannel channel, long size, boolean shared) throws IOException { 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 50a960bfa..7f64b200f 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 @@ -476,7 +476,7 @@ public void touchArtifact(RepositorySystemSession session, UpdateCheck updates); + + /** + * Deletes the specified properties file, if exists. If file existed and was deleted, returns {@code true}. + * + * @since 1.9.25 + */ + boolean delete(File file); } From 17e0a4287833e7958e9dfd4f939ead102e481281 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 27 Nov 2025 19:54:55 +0100 Subject: [PATCH 2/6] Hmm --- .../aether/internal/impl/DefaultTrackingFileManager.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 2fc03bfde..34b6d4105 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 @@ -122,8 +122,7 @@ public boolean delete(File file) { synchronized (mutex(file)) { try (RandomAccessFile raf = new RandomAccessFile(file, "rw"); FileLock unused = fileLock(raf.getChannel(), Math.max(1, raf.length()), false)) { - Files.delete(file.toPath()); - return true; + return file.delete(); } catch (NoSuchFileException e) { LOGGER.debug("No such file to delete {}: {}", file, e.getMessage()); } catch (IOException e) { From d6a607eeb344bc460a7a0598a33a3d6d97c3e25b Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 27 Nov 2025 21:43:14 +0100 Subject: [PATCH 3/6] Port back and align with master. --- .../impl/DefaultTrackingFileManager.java | 71 ++++++++++--------- .../impl/DefaultTrackingFileManagerTest.java | 13 ++++ 2 files changed, 50 insertions(+), 34 deletions(-) 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 34b6d4105..6cd9e0e42 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 @@ -21,18 +21,19 @@ import javax.inject.Named; import javax.inject.Singleton; -import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.File; -import java.io.FileInputStream; import java.io.IOException; -import java.io.RandomAccessFile; import java.io.UncheckedIOException; +import java.nio.ByteBuffer; +import java.nio.channels.Channels; 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; import java.util.Properties; @@ -54,17 +55,18 @@ public final class DefaultTrackingFileManager implements TrackingFileManager { @Override public Properties read(File file) { - if (Files.isReadable(file.toPath())) { - synchronized (mutex(file)) { - try (FileInputStream stream = new FileInputStream(file); - FileLock unused = fileLock(stream.getChannel(), Math.max(1, file.length()), true)) { + Path path = file.toPath(); + if (Files.isReadable(path)) { + synchronized (mutex(path)) { + try (FileChannel fileChannel = FileChannel.open(path, StandardOpenOption.READ); + FileLock unused = fileLock(fileChannel, Math.max(1, fileChannel.size()), true)) { Properties props = new Properties(); - props.load(stream); + props.load(Channels.newInputStream(fileChannel)); return props; } catch (NoSuchFileException e) { - LOGGER.debug("No such file to read {}: {}", file, e.getMessage()); + LOGGER.debug("No such file to read {}: {}", path, e.getMessage()); } catch (IOException e) { - LOGGER.warn("Failed to read tracking file '{}'", file, e); + LOGGER.warn("Failed to read tracking file '{}'", path, e); throw new UncheckedIOException(e); } } @@ -74,20 +76,20 @@ public Properties read(File file) { @Override public Properties update(File file, Map updates) { + Path path = file.toPath(); try { - Files.createDirectories(file.getParentFile().toPath()); + Files.createDirectories(path.getParent()); } catch (IOException e) { - LOGGER.warn("Failed to create tracking file parent '{}'", file, e); + LOGGER.warn("Failed to create tracking file parent '{}'", path, e); throw new UncheckedIOException(e); } Properties props = new Properties(); - synchronized (mutex(file)) { - try (RandomAccessFile raf = new RandomAccessFile(file, "rw"); - FileLock unused = fileLock(raf.getChannel(), Math.max(1, raf.length()), false)) { - if (raf.length() > 0) { - byte[] buffer = new byte[(int) raf.length()]; - raf.readFully(buffer); - props.load(new ByteArrayInputStream(buffer)); + synchronized (mutex(path)) { + try (FileChannel fileChannel = FileChannel.open( + path, StandardOpenOption.READ, StandardOpenOption.WRITE, StandardOpenOption.CREATE); + FileLock unused = fileLock(fileChannel, Math.max(1, fileChannel.size()), false)) { + if (fileChannel.size() > 0) { + props.load(Channels.newInputStream(fileChannel)); } for (Map.Entry update : updates.entrySet()) { @@ -98,35 +100,36 @@ public Properties update(File file, Map updates) { } } - LOGGER.debug("Writing tracking file '{}'", file); + 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."); - raf.seek(0L); - raf.write(stream.toByteArray()); - raf.setLength(raf.getFilePointer()); + fileChannel.position(0); + int written = fileChannel.write(ByteBuffer.wrap(stream.toByteArray())); + fileChannel.truncate(written); } catch (IOException e) { - LOGGER.warn("Failed to write tracking file '{}'", file, e); + LOGGER.warn("Failed to write tracking file '{}'", path, e); throw new UncheckedIOException(e); } } - return props; } @Override public boolean delete(File file) { - if (Files.isReadable(file.toPath())) { - synchronized (mutex(file)) { - try (RandomAccessFile raf = new RandomAccessFile(file, "rw"); - FileLock unused = fileLock(raf.getChannel(), Math.max(1, raf.length()), false)) { - return file.delete(); + Path path = file.toPath(); + if (Files.isReadable(path)) { + synchronized (mutex(path)) { + try (FileChannel fileChannel = FileChannel.open(path, StandardOpenOption.WRITE); + FileLock unused = fileLock(fileChannel, Math.max(1, fileChannel.size()), false)) { + Files.delete(path); + return true; } catch (NoSuchFileException e) { - LOGGER.debug("No such file to delete {}: {}", file, e.getMessage()); + LOGGER.debug("No such file to delete {}: {}", path, e.getMessage()); } catch (IOException e) { - LOGGER.warn("Failed to delete tracking file '{}'", file, e); + LOGGER.warn("Failed to delete tracking file '{}'", path, e); throw new UncheckedIOException(e); } } @@ -134,10 +137,10 @@ public boolean delete(File file) { return false; } - private Object mutex(File file) { + private Object mutex(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 ¯\_(ツ)_/¯ - return file.toPath().toAbsolutePath().normalize().toString().intern(); + return path.toAbsolutePath().normalize().toString().intern(); } private FileLock fileLock(FileChannel channel, long size, boolean shared) throws IOException { 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 7209ed798..035b96a1d 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 @@ -20,6 +20,7 @@ import java.io.File; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -31,6 +32,7 @@ import org.junit.Test; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -104,6 +106,17 @@ public void testUpdateNoFileLeak() throws Exception { } } + @Test + public void testDeleteFileIsGone() throws Exception { + TrackingFileManager tfm = new DefaultTrackingFileManager(); + + for (int i = 0; i < 1000; i++) { + File propFile = TestFileUtils.createTempFile("#COMMENT\nkey1=value1\nkey2 : value2"); + assertTrue(tfm.delete(propFile)); + assertFalse("File is not gone", Files.isRegularFile(propFile.toPath())); + } + } + @Test public void testLockingOnCanonicalPath() throws Exception { final TrackingFileManager tfm = new DefaultTrackingFileManager(); From fee06a45b465a2b6247ab51722ff8b2c1937275f Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 28 Nov 2025 13:21:09 +0100 Subject: [PATCH 4/6] Port over master changes. --- .../internal/impl/DefaultTrackingFileManager.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) 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 6cd9e0e42..59f667a8c 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 @@ -47,6 +47,9 @@ * to back off two parallel implementations that coexist in Maven (this class and {@code maven-compat} one), as in * certain cases the two implementations may collide on properties files. This locking must remain in place for as long * as {@code maven-compat} code exists. + * + * IMPORTANT: This class is kept fully in sync with the master branch one (w/ simple change to convert File + * to Path instances). */ @Singleton @Named @@ -59,7 +62,7 @@ public Properties read(File file) { if (Files.isReadable(path)) { synchronized (mutex(path)) { try (FileChannel fileChannel = FileChannel.open(path, StandardOpenOption.READ); - FileLock unused = fileLock(fileChannel, Math.max(1, fileChannel.size()), true)) { + FileLock unused = fileLock(fileChannel, true)) { Properties props = new Properties(); props.load(Channels.newInputStream(fileChannel)); return props; @@ -86,8 +89,8 @@ public Properties update(File file, Map updates) { Properties props = new Properties(); synchronized (mutex(path)) { try (FileChannel fileChannel = FileChannel.open( - path, StandardOpenOption.READ, StandardOpenOption.WRITE, StandardOpenOption.CREATE); - FileLock unused = fileLock(fileChannel, Math.max(1, fileChannel.size()), false)) { + path, StandardOpenOption.READ, StandardOpenOption.WRITE, StandardOpenOption.CREATE); + FileLock unused = fileLock(fileChannel, false)) { if (fileChannel.size() > 0) { props.load(Channels.newInputStream(fileChannel)); } @@ -123,7 +126,7 @@ public boolean delete(File file) { if (Files.isReadable(path)) { synchronized (mutex(path)) { try (FileChannel fileChannel = FileChannel.open(path, StandardOpenOption.WRITE); - FileLock unused = fileLock(fileChannel, Math.max(1, fileChannel.size()), false)) { + FileLock unused = fileLock(fileChannel, false)) { Files.delete(path); return true; } catch (NoSuchFileException e) { @@ -143,11 +146,11 @@ 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) { From 0a45a52aea13f76cfe6f1f5c3190937860e52451 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 28 Nov 2025 13:24:25 +0100 Subject: [PATCH 5/6] Reformat --- .../aether/internal/impl/DefaultTrackingFileManager.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 59f667a8c..95717edfd 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 @@ -62,7 +62,7 @@ public Properties read(File file) { if (Files.isReadable(path)) { synchronized (mutex(path)) { try (FileChannel fileChannel = FileChannel.open(path, StandardOpenOption.READ); - FileLock unused = fileLock(fileChannel, true)) { + FileLock unused = fileLock(fileChannel, true)) { Properties props = new Properties(); props.load(Channels.newInputStream(fileChannel)); return props; @@ -89,8 +89,8 @@ public Properties update(File file, Map updates) { Properties props = new Properties(); synchronized (mutex(path)) { try (FileChannel fileChannel = FileChannel.open( - path, StandardOpenOption.READ, StandardOpenOption.WRITE, StandardOpenOption.CREATE); - FileLock unused = fileLock(fileChannel, false)) { + path, StandardOpenOption.READ, StandardOpenOption.WRITE, StandardOpenOption.CREATE); + FileLock unused = fileLock(fileChannel, false)) { if (fileChannel.size() > 0) { props.load(Channels.newInputStream(fileChannel)); } @@ -126,7 +126,7 @@ public boolean delete(File file) { if (Files.isReadable(path)) { synchronized (mutex(path)) { try (FileChannel fileChannel = FileChannel.open(path, StandardOpenOption.WRITE); - FileLock unused = fileLock(fileChannel, false)) { + FileLock unused = fileLock(fileChannel, false)) { Files.delete(path); return true; } catch (NoSuchFileException e) { From 7bb4617333cf310393ccf62fb0b7fdc11584cf89 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 28 Nov 2025 14:59:50 +0100 Subject: [PATCH 6/6] PR comment --- .../aether/internal/impl/DefaultTrackingFileManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 95717edfd..7076f38f7 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 @@ -86,11 +86,11 @@ public Properties update(File file, Map updates) { LOGGER.warn("Failed to create tracking file parent '{}'", path, e); throw new UncheckedIOException(e); } - Properties props = new Properties(); 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)); } @@ -112,12 +112,12 @@ public Properties update(File file, Map updates) { 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