From ab2f4c5ebbabbf4d5c94a09143a220e085abf1f0 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Mon, 12 Oct 2026 18:54:27 +0000 Subject: [PATCH 1/2] Fix #2204: Windows AccessDeniedException on install when IDE holds jar open On Windows, Files.move() with ATOMIC_MOVE or REPLACE_EXISTING calls MoveFileEx / ReplaceFile, which require exclusive access to the destination. When an IDE (e.g. IntelliJ) keeps the installed .jar open for reading, all rename attempts in retryingMove() fail with AccessDeniedException, breaking the install. Pre-2.0.24 behavior used classicCopy() (stream-based overwrite), which succeeds on Windows even if another process holds a shared read handle, because stream opens use FILE_SHARE_READ semantics. Fix: after exhausting all WINDOWS_MOVE_ATTEMPTS with AccessDeniedException, fall back to classicCopy() directly to the target as a last resort. A WARN log identifies the process holding the lock. The atomicity guarantee is lost for this edge case, but the build completes. Set -Dorg.eclipse.aether.spi.io.PathProcessor.WINDOWS_CLASSIC_COPY_FALLBACK=false to disable the fallback and propagate the original AccessDeniedException instead. Fixes #2204 Fixes apache/maven#13329 Co-authored-by: Claude Sonnet 4.6 --- maven-resolver-spi/pom.xml | 9 +++ .../aether/spi/io/PathProcessorSupport.java | 73 +++++++++++++++---- .../spi/io/PathProcessorSupportTest.java | 31 +++++++- 3 files changed, 99 insertions(+), 14 deletions(-) diff --git a/maven-resolver-spi/pom.xml b/maven-resolver-spi/pom.xml index 210230d60..6a91ad9d4 100644 --- a/maven-resolver-spi/pom.xml +++ b/maven-resolver-spi/pom.xml @@ -43,6 +43,10 @@ org.apache.maven.resolver maven-resolver-api + + org.slf4j + slf4j-api + com.google.code.gson gson @@ -52,6 +56,11 @@ junit-jupiter-api test + + org.slf4j + slf4j-simple + test + diff --git a/maven-resolver-spi/src/main/java/org/eclipse/aether/spi/io/PathProcessorSupport.java b/maven-resolver-spi/src/main/java/org/eclipse/aether/spi/io/PathProcessorSupport.java index 703508c74..0358b336e 100644 --- a/maven-resolver-spi/src/main/java/org/eclipse/aether/spi/io/PathProcessorSupport.java +++ b/maven-resolver-spi/src/main/java/org/eclipse/aether/spi/io/PathProcessorSupport.java @@ -36,6 +36,9 @@ import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.atomic.AtomicBoolean; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import static java.util.Objects.requireNonNull; /** @@ -45,6 +48,8 @@ * @since 2.0.13 */ public class PathProcessorSupport implements PathProcessor { + protected static final Logger LOGGER = LoggerFactory.getLogger(PathProcessorSupport.class); + /** * Logic borrowed from Commons-Lang3: we really need only this, to decide do we NIO2 file ops or not. * On Windows the final move needs retry/staging logic, see {@link #retryingMove(Path, Path, StandardCopyOption[])}. @@ -71,6 +76,18 @@ public class PathProcessorSupport implements PathProcessor { protected static final long WINDOWS_MOVE_RETRY_DELAY = Long.getLong(PathProcessor.class.getName() + "WINDOWS_MOVE_RETRY_DELAY", 50L); + /** + * Whether to fall back to a direct (non-atomic) stream copy when all rename attempts on Windows are exhausted + * with {@link AccessDeniedException} due to a persistent file lock (for example an IDE holding the target + * {@code .jar} open). When {@code true} (the default), the installer degrades gracefully to the pre-2.0.24 + * behavior — the target is overwritten in-place via a stream, which on Windows succeeds even if another + * process holds a shared read handle. The atomicity guarantee is lost for this last-resort path, but the + * build succeeds. Set to {@code false} to keep the strict never-truncate contract and let the build fail + * with the original {@link AccessDeniedException} instead. + */ + protected static final boolean WINDOWS_CLASSIC_COPY_FALLBACK = Boolean.parseBoolean( + System.getProperty(PathProcessor.class.getName() + "WINDOWS_CLASSIC_COPY_FALLBACK", "true")); + @Override public boolean setLastModified(Path path, long value) throws IOException { try { @@ -249,18 +266,22 @@ public void close() throws IOException { } /** - * Moves the source file to the target path without ever truncating the target in place: the content visible at - * the target path is either the old file or the complete new file, never a partially written one. + * Moves the source file to the target path, preferring atomic rename over truncating the target in place. + *

+ * Normally the content visible at the target path is either the old file or the complete new file, never a + * partially written one. This method attempts the rename a bounded number of times ({@link #WINDOWS_MOVE_ATTEMPTS}; + * file locking by virus scanners, indexers or concurrent readers is transient on Windows), and if the file system + * refuses an atomic move (for example when source and target are on different stores), the source is first staged + * into a collocated temporary file next to the target and then renamed into place. *

- * Historically, on Windows the final move was implemented by opening the target path with a truncating stream - * and copying the source into it ({@link #classicCopy(Path, Path)}). That left a window during which a - * concurrent reader (for example a forked JVM resolving the same artifact) or an untimely process kill would - * observe, or durably leave behind, a truncated file at the final path - after checksum validation has already - * happened, so the torn content would from then on be trusted as verified. Instead, this method attempts the - * rename a bounded number of times ({@link #WINDOWS_MOVE_ATTEMPTS}, file locking by virus scanners, indexers or - * concurrent readers is transient on Windows), and if the file system refuses an atomic move (for example when - * source and target are on different stores), the source is first staged into a collocated temporary file next - * to the target and then renamed into place - the target path itself is never opened for writing. + * Last-resort fallback (Windows / IDE lock): When all rename attempts are exhausted with + * {@link AccessDeniedException} — typically because an IDE such as IntelliJ keeps the target {@code .jar} open + * continuously — and {@link #WINDOWS_CLASSIC_COPY_FALLBACK} is {@code true} (the default), the method falls back + * to a direct stream copy over the target path ({@link #classicCopy(Path, Path)}). On Windows, opening a file for + * writing via a stream succeeds even if another process holds a shared read handle, so the build completes. The + * atomicity guarantee is lost for this edge case: a concurrent reader may observe a partially written file. Set + * the system property {@code org.eclipse.aether.spi.io.PathProcessor.WINDOWS_CLASSIC_COPY_FALLBACK=false} to + * disable this fallback and propagate the {@link AccessDeniedException} instead. */ protected void retryingMove(Path source, Path target, StandardCopyOption[] copyOptions) throws IOException { FileSystemException lastException = null; @@ -297,6 +318,23 @@ protected void retryingMove(Path source, Path target, StandardCopyOption[] copyO Files.deleteIfExists(staged); } } + // All rename attempts failed with AccessDeniedException (persistent lock, e.g. IDE holding the jar open). + // As a last resort, fall back to a direct stream copy over the target. On Windows this succeeds even when + // another process holds a shared read handle because the stream open uses FILE_SHARE_READ semantics, + // unlike MoveFileEx/ReplaceFile which require exclusive access. The atomicity guarantee is lost here. + if (isClassicCopyFallbackEnabled() && (lastException instanceof AccessDeniedException)) { + LOGGER.warn( + "Atomic rename of {} to {} failed after {} attempt(s) ({}); falling back to direct copy." + + " Another process may be holding the target file open." + + " Set -D{}WINDOWS_CLASSIC_COPY_FALLBACK=false to disable this fallback.", + source, + target, + WINDOWS_MOVE_ATTEMPTS, + lastException.getMessage(), + PathProcessor.class.getName()); + classicCopy(source, target); + return; + } throw lastException; } @@ -308,8 +346,17 @@ protected void fileSystemMove(Path source, Path target, StandardCopyOption... co } /** - * Pre-NIO2 way to copy files. Important: this method must never be pointed at a "final" (published) path, as it - * opens the target with truncation; callers stage into a temporary file and rename into place instead. + * Returns whether the classic-copy fallback is enabled. Exposed as a protected method to allow subclasses + * (and tests) to override the behavior controlled by {@link #WINDOWS_CLASSIC_COPY_FALLBACK}. + */ + protected boolean isClassicCopyFallbackEnabled() { + return WINDOWS_CLASSIC_COPY_FALLBACK; + } + + /** + * Pre-NIO2 way to copy files. Opens the target with truncation; normally callers stage into a temporary file and + * rename into place to avoid exposing partially written content. Used as a last resort on Windows when all atomic + * rename attempts are exhausted due to a persistent file lock (see {@link #retryingMove}). */ protected void classicCopy(Path source, Path target) throws IOException { ByteBuffer buffer = ByteBuffer.allocate(1024 * 32); diff --git a/maven-resolver-spi/src/test/java/org/eclipse/aether/spi/io/PathProcessorSupportTest.java b/maven-resolver-spi/src/test/java/org/eclipse/aether/spi/io/PathProcessorSupportTest.java index ef845a1d5..24d165970 100644 --- a/maven-resolver-spi/src/test/java/org/eclipse/aether/spi/io/PathProcessorSupportTest.java +++ b/maven-resolver-spi/src/test/java/org/eclipse/aether/spi/io/PathProcessorSupportTest.java @@ -104,8 +104,9 @@ protected void fileSystemMove(Path source, Path target, StandardCopyOption... co } @Test - void retryingMoveNeverTruncatesTargetOnFailure(@TempDir Path tempDir) throws IOException { + void retryingMoveNeverTruncatesTargetOnFailureWhenFallbackDisabled(@TempDir Path tempDir) throws IOException { AtomicInteger attempts = new AtomicInteger(); + // Subclass that disables the classic-copy fallback so the strict no-truncate guarantee is maintained PathProcessorSupport subject = new PathProcessorSupport() { @Override protected void fileSystemMove(Path source, Path target, StandardCopyOption... copyOptions) @@ -113,6 +114,11 @@ protected void fileSystemMove(Path source, Path target, StandardCopyOption... co attempts.incrementAndGet(); throw new AccessDeniedException(target.toString()); } + + @Override + protected boolean isClassicCopyFallbackEnabled() { + return false; + } }; Path source = write(tempDir, "source.txt", "new content"); Path target = write(tempDir, "target.txt", "old content"); @@ -126,6 +132,29 @@ protected void fileSystemMove(Path source, Path target, StandardCopyOption... co assertEquals("new content", read(source)); } + @Test + void retryingMoveFallsBackToClassicCopyOnPersistentLock(@TempDir Path tempDir) throws IOException { + AtomicInteger attempts = new AtomicInteger(); + // Simulate an IDE holding the target .jar open — all rename attempts fail with AccessDeniedException. + // The classic-copy fallback is enabled (default), so the build should complete. + PathProcessorSupport subject = new PathProcessorSupport() { + @Override + protected void fileSystemMove(Path source, Path target, StandardCopyOption... copyOptions) + throws IOException { + attempts.incrementAndGet(); + throw new AccessDeniedException(target.toString()); + } + }; + Path source = write(tempDir, "source.txt", "new content"); + Path target = write(tempDir, "target.txt", "old content"); + + // Should succeed via the classic-copy fallback, not throw + subject.retryingMove(source, target, ATOMIC_OPTIONS); + + assertEquals(PathProcessorSupport.WINDOWS_MOVE_ATTEMPTS, attempts.get(), "all retry attempts must be made"); + assertEquals("new content", read(target), "target must contain the new content after classic-copy fallback"); + } + @Test void retryingMoveStagesCollocatedCopyWhenAtomicMoveNotSupported(@TempDir Path tempDir) throws IOException { PathProcessorSupport subject = new PathProcessorSupport() { From 8193c905b3608e2ecabc621e47b27ee93b8a171c Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Mon, 12 Oct 2026 20:26:26 +0000 Subject: [PATCH 2/2] Address review: fix missing dot in system property names, make LOGGER private --- .../eclipse/aether/spi/io/PathProcessorSupport.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/maven-resolver-spi/src/main/java/org/eclipse/aether/spi/io/PathProcessorSupport.java b/maven-resolver-spi/src/main/java/org/eclipse/aether/spi/io/PathProcessorSupport.java index 0358b336e..568064413 100644 --- a/maven-resolver-spi/src/main/java/org/eclipse/aether/spi/io/PathProcessorSupport.java +++ b/maven-resolver-spi/src/main/java/org/eclipse/aether/spi/io/PathProcessorSupport.java @@ -48,7 +48,7 @@ * @since 2.0.13 */ public class PathProcessorSupport implements PathProcessor { - protected static final Logger LOGGER = LoggerFactory.getLogger(PathProcessorSupport.class); + private static final Logger LOGGER = LoggerFactory.getLogger(PathProcessorSupport.class); /** * Logic borrowed from Commons-Lang3: we really need only this, to decide do we NIO2 file ops or not. @@ -61,20 +61,20 @@ public class PathProcessorSupport implements PathProcessor { * Escape hatch if atomic move is not desired on system we run on. */ protected static final boolean ATOMIC_MOVE = - Boolean.parseBoolean(System.getProperty(PathProcessor.class.getName() + "ATOMIC_MOVE", "true")); + Boolean.parseBoolean(System.getProperty(PathProcessor.class.getName() + ".ATOMIC_MOVE", "true")); /** * The number of attempts for the final move on Windows, where the move target may be transiently locked by a * virus scanner, an indexer or a concurrent reader. */ protected static final int WINDOWS_MOVE_ATTEMPTS = - Math.max(1, Integer.getInteger(PathProcessor.class.getName() + "WINDOWS_MOVE_ATTEMPTS", 5)); + Math.max(1, Integer.getInteger(PathProcessor.class.getName() + ".WINDOWS_MOVE_ATTEMPTS", 5)); /** * The delay in milliseconds applied between the move attempts on Windows. */ protected static final long WINDOWS_MOVE_RETRY_DELAY = - Long.getLong(PathProcessor.class.getName() + "WINDOWS_MOVE_RETRY_DELAY", 50L); + Long.getLong(PathProcessor.class.getName() + ".WINDOWS_MOVE_RETRY_DELAY", 50L); /** * Whether to fall back to a direct (non-atomic) stream copy when all rename attempts on Windows are exhausted @@ -86,7 +86,7 @@ public class PathProcessorSupport implements PathProcessor { * with the original {@link AccessDeniedException} instead. */ protected static final boolean WINDOWS_CLASSIC_COPY_FALLBACK = Boolean.parseBoolean( - System.getProperty(PathProcessor.class.getName() + "WINDOWS_CLASSIC_COPY_FALLBACK", "true")); + System.getProperty(PathProcessor.class.getName() + ".WINDOWS_CLASSIC_COPY_FALLBACK", "true")); @Override public boolean setLastModified(Path path, long value) throws IOException { @@ -326,7 +326,7 @@ protected void retryingMove(Path source, Path target, StandardCopyOption[] copyO LOGGER.warn( "Atomic rename of {} to {} failed after {} attempt(s) ({}); falling back to direct copy." + " Another process may be holding the target file open." - + " Set -D{}WINDOWS_CLASSIC_COPY_FALLBACK=false to disable this fallback.", + + " Set -D{}.WINDOWS_CLASSIC_COPY_FALLBACK=false to disable this fallback.", source, target, WINDOWS_MOVE_ATTEMPTS,