From f68f1d79ae33998b4f9f915db07b1550bd5b1d6d Mon Sep 17 00:00:00 2001
From: Guillaume Nodet <gnodet@gmail.com>
Date: Fri, 25 Jul 2025 06:39:00 +0000
Subject: [PATCH] Fix recursive update issue in SmartProjectComparator

Fixes #10995

The SmartProjectComparator.getProjectWeight() method was using
ConcurrentHashMap.computeIfAbsent() in a recursive context, which
could lead to IllegalStateException: Recursive update when calculating
project weights in complex dependency graphs or concurrent scenarios.

Changes:
- Replace computeIfAbsent() with explicit get() + putIfAbsent() pattern
- Eliminate recursive calls to computeIfAbsent() that violate
  ConcurrentHashMap's internal constraints
- Maintain thread safety using putIfAbsent() for concurrent access
- Add comprehensive test case to verify the fix under concurrent load

The fix preserves all existing functionality while eliminating the
recursive update exception that could occur during parallel builds
of large multi-module projects.
---
 .../multithreaded/SmartProjectComparator.java | 13 ++++-
 .../SmartProjectComparatorTest.java           | 54 +++++++++++++++++++
 2 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/multithreaded/SmartProjectComparator.java b/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/multithreaded/SmartProjectComparator.java
index 8f5626f4ecb2..829c6e4df782 100644
--- a/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/multithreaded/SmartProjectComparator.java
+++ b/impl/maven-core/src/main/java/org/apache/maven/lifecycle/internal/builder/multithreaded/SmartProjectComparator.java
@@ -80,7 +80,18 @@ public Comparator<MavenProject> getComparator() {
      * @return the project's weight (higher means longer dependency chain)
      */
     public long getProjectWeight(MavenProject project) {
-        return projectWeights.computeIfAbsent(project, this::calculateWeight);
+        // First check if weight is already calculated
+        Long existingWeight = projectWeights.get(project);
+        if (existingWeight != null) {
+            return existingWeight;
+        }
+
+        // Calculate weight without using computeIfAbsent to avoid recursive update issues
+        long weight = calculateWeight(project);
+
+        // Use putIfAbsent to handle concurrent access safely
+        Long previousWeight = projectWeights.putIfAbsent(project, weight);
+        return previousWeight != null ? previousWeight : weight;
     }
 
     private Comparator<MavenProject> createComparator() {
diff --git a/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/builder/multithreaded/SmartProjectComparatorTest.java b/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/builder/multithreaded/SmartProjectComparatorTest.java
index fc656acf5927..50cac26e1b8f 100644
--- a/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/builder/multithreaded/SmartProjectComparatorTest.java
+++ b/impl/maven-core/src/test/java/org/apache/maven/lifecycle/internal/builder/multithreaded/SmartProjectComparatorTest.java
@@ -20,6 +20,11 @@
 
 import java.util.Arrays;
 import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
 
 import org.apache.maven.execution.ProjectDependencyGraph;
 import org.apache.maven.lifecycle.internal.stub.ProjectDependencyGraphStub;
@@ -154,4 +159,53 @@ void testSameWeightOrdering() {
         long weightC = comparator.getProjectWeight(ProjectDependencyGraphStub.C);
         assertEquals(weightB, weightC, "Projects B and C should have the same weight");
     }
+
+    @Test
+    void testConcurrentWeightCalculation() throws Exception {
+        // Test that concurrent weight calculation doesn't cause recursive update issues
+        // This test simulates the scenario that causes the IllegalStateException
+
+        int numThreads = 10;
+        int numIterations = 100;
+        ExecutorService executor = Executors.newFixedThreadPool(numThreads);
+        CountDownLatch latch = new CountDownLatch(numThreads);
+        AtomicReference<Exception> exception = new AtomicReference<>();
+
+        for (int i = 0; i < numThreads; i++) {
+            executor.submit(() -> {
+                try {
+                    for (int j = 0; j < numIterations; j++) {
+                        // Simulate concurrent access to weight calculation
+                        // This can trigger the recursive update issue
+                        List<MavenProject> projects = Arrays.asList(
+                                ProjectDependencyGraphStub.A,
+                                ProjectDependencyGraphStub.B,
+                                ProjectDependencyGraphStub.C,
+                                ProjectDependencyGraphStub.X,
+                                ProjectDependencyGraphStub.Y,
+                                ProjectDependencyGraphStub.Z);
+
+                        // Sort projects concurrently - this triggers weight calculation
+                        projects.sort(comparator.getComparator());
+
+                        // Also directly access weights to increase contention
+                        for (MavenProject project : projects) {
+                            comparator.getProjectWeight(project);
+                        }
+                    }
+                } catch (Exception e) {
+                    exception.set(e);
+                } finally {
+                    latch.countDown();
+                }
+            });
+        }
+
+        latch.await(30, TimeUnit.SECONDS);
+        executor.shutdown();
+
+        if (exception.get() != null) {
+            throw exception.get();
+        }
+    }
 }