Fix concurrent builder race conditions in artifact resolution and phase ordering - #12680
Draft
gnodet wants to merge 2 commits into
Draft
Fix concurrent builder race conditions in artifact resolution and phase ordering#12680gnodet wants to merge 2 commits into
gnodet wants to merge 2 commits into
Conversation
…se ordering The concurrent builder (-b concurrent) suffered from several race conditions when building large multi-module projects like Apache Camel (682 modules): 1. Thread-safety for session.getCurrentProject(): Multiple executor threads shared MavenSession.currentProject, causing dependency resolution and artifact filter to use the wrong project. Fixed by adding a ThreadLocal in the concurrent MojoExecutor and recording lifecycle phases directly on the correct project (not via the racy session reference). 2. MavenProject artifact state thread-safety: setResolvedArtifacts(), setArtifactFilter(), getArtifacts(), and getArtifactMap() were not thread-safe. Made them synchronized with merge semantics that preserve artifact file references across concurrent resolutions. 3. Reactor output directory fallback: When a reactor dependency has been compiled but not yet packaged, use its output directory (if it exists) as the artifact file. This prevents NPEs in downstream compiler plugin classpath resolution. 4. V4 lifecycle RESOURCES ordering: Added after(SOURCES) constraint to the RESOURCES phase so resource processing happens after source generation, matching Maven 3 sequential ordering. 5. after:* step skip propagation: Changed the after:* step decision from checking before:* status to checking the phase step itself. Before:* is an empty lifecycle setup step that always completes early; checking it allowed after:* steps to execute when the phase was SKIPPED, causing downstream steps to run out of order. 6. Scope expansion for dependency ordering: filterByScope() now expands "compile" scope to include provided and system dependencies, matching Maven dependency resolution semantics and ensuring provided-scope reactor dependencies are properly ordered. Tested with Apache Camel 682-module build: reduced concurrent builder failures from 67 modules to 0 (remaining 3 failures are pre-existing Maven 4 issues unrelated to the concurrent builder). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…l-file fallbacks Address review feedback on the concurrent builder patch: - Revert MavenProject.java to upstream: remove synchronized keywords and merge logic from setResolvedArtifacts/setArtifactFilter/getArtifacts. MavenSession.currentProject is already a ThreadLocal in Maven 4, so synchronization is unnecessary. - Revert RESOURCES ordering in DefaultLifecycleRegistry: remove after(SOURCES) constraint. V4-native plugins should use @after annotations to declare ordering dependencies explicitly. - Add Maven 3 personality conditional in BuildPlanExecutor: when maven.maven3Personality is enabled, enforce sequential SOURCES → RESOURCES ordering for backward compatibility with plugins that rely on Maven 3 phase ordering. - Add null-file artifact fallbacks in LifecycleDependencyResolver: 1. GAV-based reactor lookup when Artifact.equals() misses due to type/classifier mismatch between resolved dependency and reactor artifact. 2. Local repository lookup for non-reactor artifacts whose files were not set by the Aether resolver (race condition in concurrent builder where artifacts are collected but not fully resolved). These fallbacks eliminate NPEs in the compiler plugin when it calls getCompileClasspathElements() on artifacts with null files during concurrent builds. Verified with a full Apache Camel concurrent build (-b concurrent -T1C -Dquickly): zero NPEs across all modules. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
1 participant
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.
Summary
Fixes race conditions in Maven 4.x concurrent builder (
-b concurrent) that cause NPEs during artifact resolution. Verified with a full Apache Camel concurrent build (-b concurrent -T1C -Dquickly).Root Cause
The concurrent builder can leave resolved dependency artifacts with null files in two cases:
Artifact.getFile()returns null)Plugins like
maven-compiler-plugin3.15.0 callgetCompileClasspathElements()which iterates over dependency artifacts and callsartifact.getFile().getPath()— NPE when the file is null.Changes
LifecycleDependencyResolver.java— Three fallback mechanisms for null-file artifacts:target/classes)Artifact.equals()misses due to type/classifier mismatch, match bygroupId:artifactId:versionLocalRepositoryManager.getPathForLocalArtifact()BuildPlanExecutor.java— Maven 3 personality conditional:maven.maven3Personalityis enabled, enforce sequentialSOURCES → RESOURCESordering for backward compatibility with plugins that assume Maven 3 phase ordering@Afterannotations to declare ordering dependenciesDefaultLifecycleRegistry.java— Revert RESOURCES ordering:after(SOURCES)constraint from the lifecycle definition (moved to a runtime conditional inBuildPlanExecutorgated on Maven 3 personality)Testing
Test plan
🤖 Generated with Claude Code