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..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 @@ -21,17 +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; @@ -45,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 @@ -53,15 +58,18 @@ public final class DefaultTrackingFileManager implements TrackingFileManager { @Override public Properties read(File file) { - if (Files.isReadable(file.toPath())) { - synchronized (getMutex(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, 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 {}: {}", 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); } } @@ -71,22 +79,20 @@ public Properties read(File file) { @Override public Properties update(File file, Map updates) { - Properties props = new Properties(); - + 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); } - - synchronized (getMutex(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, false)) { + Properties props = new Properties(); + if (fileChannel.size() > 0) { + props.load(Channels.newInputStream(fileChannel)); } for (Map.Entry update : updates.entrySet()) { @@ -97,46 +103,54 @@ 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); + return props; } 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) { + Path path = file.toPath(); + 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(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 ¯\_(ツ)_/¯ - /* - * 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 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 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); } 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();