Fix #11009: Prevent StackOverflowError in parent POM resolution #11106
Conversation
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
This commit addresses issue MNG-11009 where Maven would encounter a StackOverflowError when resolving parent POMs that form a cycle. Changes made: - Modified DefaultModelBuilder to detect cycles in parent POM resolution - Added cycle detection logic that tracks visited parent coordinates - When a cycle is detected, throws ModelBuildingException with clear error message - Added comprehensive integration test to verify the fix works correctly The fix prevents infinite recursion by maintaining a set of visited parent coordinates during the resolution process. When a parent that has already been visited is encountered again, it indicates a cycle and the process is terminated with an appropriate error message. Integration test includes: - Test case with cyclic parent dependencies (A -> B -> A) - Verification that StackOverflowError no longer occurs - Confirmation that appropriate cycle detection error is thrown
gnodet
changed the title
The original cycle detection implementation was incorrectly applied at the wrong level, causing normal parent hierarchies to be identified as cycles. This resulted in test failures where getEffectiveModel() returned null. Key changes: - Moved cycle detection from readParentLocally/readAsParentModel to resolveParent method - Use model IDs (groupId:artifactId:version) instead of file paths for cycle detection - Apply cycle detection only during actual parent resolution, not for every model processed - Ensure proper cleanup in finally block to remove parent ID from resolution chain This fix allows normal parent hierarchies to work correctly while still detecting actual cycles like the MNG-11009 scenario where a parent references itself through relative path resolution. Fixes failing tests: - DefaultModelBuilderTest.testPropertiesAndProfiles - RequestTraceTest.testTraces - TestApiStandalone.testStandalone
This change adds cycle detection when resolving parent POMs to prevent infinite loops that can occur when there are circular parent references. Key changes: - Added cycle detection logic in DefaultModelBuilder.readParentExternally() - Maintains a stack of currently resolving parent GAVs to detect cycles - Throws ModelBuilderException with clear error message when cycle detected - Added comprehensive test coverage in ParentCycleDetectionTest The implementation tracks parent resolution using a ThreadLocal stack to handle concurrent builds safely. When a cycle is detected, it provides a detailed error message showing the complete cycle path. Fixes potential infinite loops and stack overflow errors when processing POMs with circular parent dependencies.
…jects The parent cycle detection was previously interfering with the multi-module project test because it was being triggered when multiple modules had the same parent. This change ensures that: 1. The cycle detection still works for actual parent cycles 2. It doesn't interfere with legitimate multi-module scenarios 3. The DefaultMavenTest.testThatErrorDuringProjectDependencyGraphCreationAreStored test now passes correctly The key fix was ensuring that the cycle detection properly handles the parent chain cleanup and doesn't create false positives when multiple modules reference the same parent POM. Also updated the ParentCycleDetectionTest to use proper Maven 4.1.0 namespaces in the test POMs.
|
kindly asking if this is a connection/config issue or something code-related, as it is reproducible on my system as well. |
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.
This PR addresses issue #11009 where Maven would encounter a StackOverflowError when resolving parent POMs that form a cycle.
Problem
When Maven encounters a cyclic dependency in parent POM resolution (e.g., A -> B -> A), it would enter an infinite recursion loop, eventually causing a StackOverflowError. This made Maven crash ungracefully without providing useful feedback to the user.
Solution
The fix implements cycle detection in the
DefaultModelBuilderby:ModelBuildingExceptionwith a clear error message instead of crashingChanges Made
DefaultModelBuilder: Added cycle detection logic in parent POM resolution methodsTesting
The fix includes a comprehensive integration test (
MavenITmng11009StackOverflowParentResolutionTest) that:Backward Compatibility
This change is backward compatible as it only affects error handling for invalid POM structures. Valid POMs continue to work as before, while invalid cyclic structures now fail gracefully instead of crashing.
Related Issues