Make projectsWithDeployExecution cache initialization atomic under concurrent first access - #2
Draft
java-dependency-upgrade-fixer[bot] wants to merge 1 commit into
Conversation
…ncurrent first access
1 of 2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
DeployMojo caches the reactor-wide
projectsWithDeployExecutionlist in the first reactor project's plugin context, but initialized it with a plain get/null-check/full-scan/put sequence. Under parallel module mojo execution, multiple callers could miss the cache simultaneously and each repeat the full O(N) reactor scan, undermining the promised O(N)-total bound and relying on unspecified concurrent-mutation semantics of the context map.Evidence
Decompiling the exact
maven-core:4.0.0-rc-5artifact pinned in this plugin'spom.xmlshowsMavenSession#getPluginContextbacks the per-project plugin context with ajava.util.concurrent.ConcurrentHashMap(via nestedConcurrentMap#computeIfAbsentcalls).ConcurrentHashMap#computeIfAbsentguarantees the mapping function executes at most once per key, atomically, even when invoked concurrently by multiple threads.Fix
Replaced the manual get/null-check/scan/put sequence in
DeployMojo.getProjectsWithDeployExecution()with a singlectx.computeIfAbsent(PROJECTS_WITH_DEPLOY_KEY, key -> ...)call. This is the minimal change: same key, same first-project context ownership, same filter/order, same empty-reactor fast path, same exception propagation (an exception thrown while computing records no mapping, matching the original's behavior of only callingputafter a successful scan).Proof
DeployMojoDeployExecutionCacheTest: locks the sequential, single-threaded contract (empty reactor, filtering/order, cache reuse across repeated calls). Passes identically before and after the production edit (mvn -o test -Dtest=DeployMojoDeployExecutionCacheTest,DeployMojoTest,DeployMojoPomPackagingTest, 16/16 both times).DeployMojoConcurrentInitializationTest: a deterministic barrier/counting harness with 16 threads racing over an 8-project reactor, backed by a realConcurrentHashMapand a 200ms delay on the first project's build lookup to widen the race window. Against the original PR-head code this failed 5/5 repeated runs (concurrent callers received distinct list instances, i.e. multiple full scans). Against the candidate it passed 15/15 runs across 3 batches (exactly one full scan, one shared cached instance observed by every caller).mvn -o verify: BUILD SUCCESS, 0 Checkstyle violations, Rat check clean, 29/29 tests passing.Limitations
The fix's atomicity relies on
Session#getPluginContextreturning aConcurrentHashMap, which is a verified implementation detail of the currently pinnedmaven-core:4.0.0-rc-5, not a documented contract of the abstractorg.apache.maven.api.Sessioninterface. This is no less safe than the original PR's own manual pattern, which relied on the same unstated assumption without any atomicity guarantee at all.Generated by JAIPilot Cloud for #1 from Anthropic session
sesn_018YddSakpSpcJfY86Nh2nmT.