Skip to content

Make projectsWithDeployExecution cache initialization atomic under concurrent first access - #2

Draft
java-dependency-upgrade-fixer[bot] wants to merge 1 commit into
evaluation/maven-deploy-pr-684from
jaipilot/pr-1-cJfY86Nh2nmT
Draft

java-dependency-upgrade-fixer[bot] wants to merge 1 commit into
evaluation/maven-deploy-pr-684from
jaipilot/pr-1-cJfY86Nh2nmT

Conversation

@java-dependency-upgrade-fixer

Copy link
Copy Markdown

Problem

DeployMojo caches the reactor-wide projectsWithDeployExecution list 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-5 artifact pinned in this plugin's pom.xml shows MavenSession#getPluginContext backs the per-project plugin context with a java.util.concurrent.ConcurrentHashMap (via nested ConcurrentMap#computeIfAbsent calls). ConcurrentHashMap#computeIfAbsent guarantees 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 single ctx.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 calling put after a successful scan).

Proof

  • Added 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).
  • Added DeployMojoConcurrentInitializationTest: a deterministic barrier/counting harness with 16 threads racing over an 8-project reactor, backed by a real ConcurrentHashMap and 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).
  • Final mvn -o verify: BUILD SUCCESS, 0 Checkstyle violations, Rat check clean, 29/29 tests passing.

Limitations

The fix's atomicity relies on Session#getPluginContext returning a ConcurrentHashMap, which is a verified implementation detail of the currently pinned maven-core:4.0.0-rc-5, not a documented contract of the abstract org.apache.maven.api.Session interface. 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.

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.

0 participants