Fix #2204: Windows AccessDeniedException on install when IDE holds jar open

[gnodet]

Summary

Fixes a Windows regression introduced by PR #2083 (resolver 2.0.24 / Maven 3.10.0): mvn install throws java.nio.file.AccessDeniedException when an IDE (e.g. IntelliJ IDEA) keeps the installed .jar open.

Closes #2204 · Closes apache/maven#13329

Root Cause

PR #2083 replaced the old stream-based file copy with Files.move() + ATOMIC_MOVE in PathProcessorSupport.retryingMove() to fix the f007 security finding. That was the right thing to do — but it introduced a behavioral regression on Windows.

Old behavior (resolver ≤ 1.9.x): classicCopy() opens the target with a truncating FileOutputStream. On Windows, this succeeds even when another process has the file open for reading, because the stream open uses FILE_SHARE_READ semantics.

New behavior (resolver 2.0.24): Files.move() maps to MoveFileEx/ReplaceFile on Windows. These system calls require exclusive access to the destination — any open handle (including a shared read-only handle from an IDE's class loader) causes AccessDeniedException. With only 5 retries at 50 ms (250 ms total), a persistent IDE lock is never released and all retries fail.

Operation	Target open by reader?	Succeeds on Windows?
FileOutputStream(target) (stream write)	Yes (shared read)	✅ Yes
Files.move(src, target, ATOMIC_MOVE)	Yes (shared read)	❌ No — needs exclusive

Fix

After exhausting all WINDOWS_MOVE_ATTEMPTS with AccessDeniedException, fall back to classicCopy() directly to the target as a last resort, logging a WARN to identify the situation. The atomicity guarantee is lost for this edge case (a concurrent reader may observe a partially written file), but the build completes — matching the pre-2.0.24 behavior that users relied on.

A new system property controls the fallback:

-Dorg.eclipse.aether.spi.io.PathProcessor.WINDOWS_CLASSIC_COPY_FALLBACK=false

Setting it to false disables the fallback and propagates the original AccessDeniedException (strict never-truncate mode).

The fallback is also exposed as an overridable protected boolean isClassicCopyFallbackEnabled() method for subclasses and tests.

Note on PR #2198

The open PR #2198 (hard-link install) sidesteps this problem for mvn install by not copying at all. This PR fixes the underlying retryingMove logic, which is also exercised in other code paths (download to local repo, writeFile). The two PRs are complementary.

Changes

• maven-resolver-spi/pom.xml — add slf4j-api compile dependency and slf4j-simple test dependency
• PathProcessorSupport.java — add WINDOWS_CLASSIC_COPY_FALLBACK constant, isClassicCopyFallbackEnabled() hook, fallback logic in retryingMove()
• PathProcessorSupportTest.java — rename retryingMoveNeverTruncatesTargetOnFailure → ...WhenFallbackDisabled (adds subclass override), add retryingMoveFallsBackToClassicCopyOnPersistentLock

Hermès Agent (Claude Sonnet 4.6) on behalf of Guillaume Nodet

[gnodet-bot]

Solid fix for a real Windows regression — thank you for this.

Bug: Javadoc documents a different property name than the code produces

The Javadoc for WINDOWS_CLASSIC_COPY_FALLBACK (line 283) reads:

{@code org.eclipse.aether.spi.io.PathProcessor.WINDOWS_CLASSIC_COPY_FALLBACK=false}

But the field is initialised as:

System.getProperty(PathProcessor.class.getName() + "WINDOWS_CLASSIC_COPY_FALLBACK", "true")

PathProcessor.class.getName() returns org.eclipse.aether.spi.io.PathProcessor (no trailing dot), so the actual property name at runtime is org.eclipse.aether.spi.io.PathProcessorWINDOWS_CLASSIC_COPY_FALLBACK — without the separating dot. A user following the Javadoc would set the wrong key and wonder why the fallback never disables.

The same pattern is used for the two pre-existing WINDOWS_MOVE_ATTEMPTS / WINDOWS_MOVE_RETRY_DELAY constants (and the warn log at line 329 also omits the dot), so this is an inherited inconsistency rather than a new mistake — but since the Javadoc here explicitly shows the full property name, the mismatch will confuse users trying to disable the fallback. Either add the dot to the + "…" concatenation (.getName() + ".WINDOWS_CLASSIC_COPY_FALLBACK") or update the Javadoc to match what the code actually produces.

Minor: Logger visibility

LOGGER is declared protected at line 51. Since no subclass in the codebase needs direct access to the logger (tests override isClassicCopyFallbackEnabled() instead), private is the conventional and safer choice. Not blocking.

Note: slf4j-api as compile-scope dep on the SPI module

Adding slf4j-api at compile scope to maven-resolver-spi makes it a transitive dependency for all consumers of the SPI. If the intent is to log only inside PathProcessorSupport (an impl class), the dependency arguably belongs on maven-resolver-impl rather than maven-resolver-spi. Worth double-checking the module boundary — the SPI is supposed to be a thin contract layer.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[Copilot]

🟡 Changes recommended

The default fallback weakens artifact publication integrity, and the documented opt-out property is not recognized.

2 open findings

• Avoid truncating fallback for atomic artifact publication · New
• Fix missing separator in fallback opt-out property · New

What changed in this PR

Adds a Windows fallback when atomic moves fail because another process holds the target open.

Changes:

• Adds configurable classic-copy fallback with warning logging.
• Adds fallback and strict-mode tests.
• Adds SLF4J dependencies.

File	Description
maven-resolver-spi/​pom.xml	Adds logging dependencies.
PathProcessorSupport.java	Implements configurable fallback behavior.
PathProcessorSupportTest.java	Tests fallback and disabled modes.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[gnodet-bot]

The two bugs I raised are fixed — thank you:

• ✅ Dot separator in system property names — all three properties now use the correct ...getName() + ".PROPERTY_NAME" form.
• ✅ Logger visibility — downgraded to private static final.
• ✅ New test coverage — retryingMoveFallsBackToClassicCopyOnPersistentLock and the renamed retryingMoveNeverTruncatesTargetOnFailureWhenFallbackDisabled cover both paths cleanly; the Template Method override for isClassicCopyFallbackEnabled() is well designed.

The open question about slf4j-api at compile scope on the SPI module remains (see inline). It has low practical impact since all downstream modules already bring their own slf4j-api, but it is still worth a conscious decision.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[cstamas]

unrelated remark: Now that SPI is pulling in logging library, we could pull it into util as well, or, just make it "official" and make API depend on slf4j? Right now, we had several "painful" decisions to not bring in logging library into api. spi and util, but it really makes sense... so to say, "integrating app" was even today forced to provide logging backend at runtime but did not have to have it on own classpath, but now does.

Explanation: so far the "surface" of resolver was clean of logging libraries, any client code depending on it (api, spi and util) could have "logging library free" classpath. Naturally, at runtime, one had to provide some for it (MIMA doco states also, that while you do not have to use/depend on slf4j, the runtime bundle MUST have it).