Skip to content

Give each mvn.cmd run its own temp directory - #13717

Merged
slachiewicz merged 1 commit into
masterfrom
agent/mvn-cmd-unique-temp
Oct 28, 2026
Merged

slachiewicz merged 1 commit into
masterfrom
agent/mvn-cmd-unique-temp

Conversation

@slachiewicz

Copy link
Copy Markdown
Member

Fixes #13716.

mvn.cmd now creates its jvm.config argfile and its mvn -v settings dump inside a directory made by a new :mkTempDir routine. mkdir fails when the name already exists, so processes that start in the same second, and therefore draw the same %RANDOM% values, retry with the next values instead of sharing a file. The directory is removed where the file used to be deleted. JVM_CONFIG_DIR and JVM_CONFIG_TEMP are cleared at the start, so a value inherited from the environment is never used or deleted.

I have no Windows machine, so this has not been run outside CI. The Windows integration-test jobs on this PR run MavenITmng4559MultipleJvmArgsTest, which failed in three of four failed master builds on 2026-10-22 and 2026-10-23; one green run does not prove a race fixed.

%RANDOM% is seeded from the clock once per cmd.exe, so mvn.cmd processes
started in the same second named their jvm.config and version temp files
alike. Since the JVM reads the jvm.config file as an @argfile and it is kept
until the JVM exits, parallel builds ran with each other's JVM arguments.
mkdir fails on an existing name, so a retry loop yields a private directory.
@slachiewicz slachiewicz added the bug Something isn't working label Oct 26, 2026
@slachiewicz
slachiewicz marked this pull request as ready for review October 26, 2026 10:37

@gnodet-bot gnodet-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verified fix for the %RANDOM% seed-collision race: the :mkTempDir subroutine uses mkdir atomicity with a 100-iteration retry loop, ensuring concurrent processes never share a temp file. All cleanup paths (normal exit, java failure, parser failure, JVM error) are covered. One nit below.

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

Comment on lines +174 to 175
rd /s /q "%VERSION_SETTINGS_DIR%" 2>nul
set "VERSION_SETTINGS_TEMP="

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Nit — stale variable on java failure path

After rd /s /q removes the directory, VERSION_SETTINGS_DIR stays defined (it still holds the now-deleted path). Only VERSION_SETTINGS_TEMP is cleared. For symmetry with the initial set "VERSION_SETTINGS_DIR=" at the top of the script, clear it here too.

Suggested change
rd /s /q "%VERSION_SETTINGS_DIR%" 2>nul
set "VERSION_SETTINGS_TEMP="
rd /s /q "%VERSION_SETTINGS_DIR%" 2>nul
set "VERSION_SETTINGS_DIR="
set "VERSION_SETTINGS_TEMP="

@slachiewicz
slachiewicz merged commit 9ad8307 into master Oct 28, 2026
23 checks passed
@slachiewicz
slachiewicz deleted the agent/mvn-cmd-unique-temp branch October 28, 2026 23:23
@github-actions github-actions Bot added this to the 4.2.0 milestone Oct 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mvn.cmd processes started in the same second share the jvm.config temp file

2 participants