Skip to content

[2.x] fix: Tolerate a locked dirzip destination on Windows - #9619

Open
jozanek wants to merge 2 commits into
sbt:developfrom
jozanek:fix/actioncache-windows-move
Open

jozanek wants to merge 2 commits into
sbt:developfrom
jozanek:fix/actioncache-windows-move

Conversation

@jozanek

@jozanek jozanek commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

On Windows, Files.move cannot replace a file that another process holds open. Under parallel task execution two tasks can install the same classes.sbtdir.zip concurrently while each also reads the zip it just installed (digest, CAS copy, extraction), so installPackagedZip fails with AccessDeniedException. Seen in the wild as recurring sbt/zinc CI failures on (compilerInterface / Compile / compileIncremental), triggered by the duplicate compilation in #9618.

The zip is now installed via IO.copyFile, which stages next to the destination and retries the atomic rename; if the destination still cannot be replaced but exists, the concurrently installed zip wins. Callers derive digests from the file on disk, so the surviving zip determines the recorded content either way. The new test races installs against readers of the installed zip and reproduces the failure on Windows without this change.

Investigated with Claude Code, with human review in the loop.

Comment on lines +380 to +389
try
Files.copy(builtZip, staging, StandardCopyOption.REPLACE_EXISTING)
try Retry(moveIntoPlace())
catch
// Windows refuses to replace a file that another process holds open. Under
// parallel task execution a concurrent install of the same directory may
// still be reading `destZip` after the retries are exhausted; accept its
// zip in that case. Callers derive digests from the file on disk, so the
// surviving install determines the recorded content either way.
case _: AccessDeniedException if Files.exists(destZip) => ()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into this. The implementation looks equivalent to IO.writeFileAtomically, so I suggest using IO.copyFile.

@ColOfAbRiX ColOfAbRiX Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A similar issue has been affecting me badly because, on one side, metals is not reliable and I can't use it or MCP, and, on the other side, sbt keeps locking files impacting manual execution and AI. The problem I have, though, is not just about parallel execution.

I've been working on a fix too, but my fix is on the read side. Let me publish my WIP solution, once I solve my env problems

@ColOfAbRiX ColOfAbRiX Aug 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it is #9627 It's still a draft as I haven't done all the checks I'd like to do and I haven't validated against the project's guidelines.

I hope this will start a good conversation around this problem

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, switched to IO.copyFile in 758c31e. The only thing kept in ActionCache is the narrow AccessDeniedException fallback: with tight-loop readers on Windows the destination can still be held open after IO.copyFile's retries are exhausted, and in that case the concurrently installed zip wins.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants