[backport 4.0.x] Fix #12592: use string-based ID for forked execution cycle detection - #12898
Conversation
…12867) The cycle detection in DefaultLifecycleExecutionPlanCalculator used Set<MojoDescriptor> (object equality) to track already-planned forked executions. Because DefaultPluginDescriptorCache clones the PluginDescriptor (and all its MojoDescriptor objects) on every cache retrieval, independently-loaded descriptors for the same plugin are always different object instances. While MojoDescriptor does override equals/hashCode, relying on mutable object equality inside a HashSet is fragile — if any identity field changes after insertion, the set lookup silently fails. Switch to Set<String> using MojoDescriptor.getId() (groupId:artifactId:version:goal) for cycle detection. String keys are immutable and make the comparison semantics explicit, eliminating any risk of identity mismatch across cloned descriptor instances. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Clean backport of #12867 to maven-4.0.x (byte-identical diffs). The change from Set<MojoDescriptor> to Set<String> using immutable MojoDescriptor.getId() keys (groupId:artifactId:version:goal) for forked execution cycle detection is correct — eliminates fragility of using mutable objects as hash set keys when DefaultPluginDescriptorCache clones descriptors on every cache retrieval.
All three cycle detection sites are consistently updated: fillMojoDescriptors (insertion), calculateForkedLifecycle (contains check), and calculateForkedGoal (contains check). The new test directly validates that independently-created descriptors with the same coordinates produce the same getId() value.
✅ LGTM — no issues found.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of Guillaume Nodet
1 participant
Backport of #12867 to maven-4.0.x.
Original PR: #12867