From 6b6f44a92ec3e3f2a87aa25f94f85474dec77aa7 Mon Sep 17 00:00:00 2001 From: elharo Date: Thu, 30 Jul 2026 13:08:18 +0000 Subject: [PATCH 1/2] Fix #12605: DefaultProjectBuildingHelper synchronized bottleneck in parallel builds Remove the synchronized keyword from createProjectRealm() and fix the check-then-act race in DefaultProjectRealmCache.put() by using ConcurrentHashMap.putIfAbsent() instead of containsKey() + put(). The projectRealmCache already uses ConcurrentHashMap and is thread-safe for get/createKey operations. The only non-thread-safe portion was the put() method's check-then-act pattern, which is now atomic. This allows multiple threads to set up extension realms for different projects concurrently rather than serializing through a global lock. Fixes gh-12605 --- .../project/DefaultProjectBuildingHelper.java | 2 +- .../project/DefaultProjectRealmCache.java | 10 +-- .../project/DefaultProjectRealmCacheTest.java | 73 +++++++++++++++++++ 3 files changed, 79 insertions(+), 6 deletions(-) create mode 100644 impl/maven-core/src/test/java/org/apache/maven/project/DefaultProjectRealmCacheTest.java diff --git a/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuildingHelper.java b/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuildingHelper.java index 6bde396a97e7..38178c389386 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuildingHelper.java +++ b/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuildingHelper.java @@ -143,7 +143,7 @@ public List createArtifactRepositories( } @Override - public synchronized ProjectRealmCache.CacheRecord createProjectRealm( + public ProjectRealmCache.CacheRecord createProjectRealm( MavenProject project, Model model, ProjectBuildingRequest request) throws PluginResolutionException, PluginVersionResolutionException, PluginManagerException { ClassRealm projectRealm; diff --git a/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectRealmCache.java b/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectRealmCache.java index 9111177c3489..262c4c2c7b96 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectRealmCache.java +++ b/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectRealmCache.java @@ -94,13 +94,13 @@ public CacheRecord get(Key key) { public CacheRecord put(Key key, ClassRealm projectRealm, DependencyFilter extensionArtifactFilter) { Objects.requireNonNull(projectRealm, "projectRealm cannot be null"); - if (cache.containsKey(key)) { - throw new IllegalStateException("Duplicate project realm for extensions " + key); - } - CacheRecord record = new CacheRecord(projectRealm, extensionArtifactFilter); - cache.put(key, record); + CacheRecord existing = cache.putIfAbsent(key, record); + + if (existing != null) { + throw new IllegalStateException("Duplicate project realm for extensions " + key); + } return record; } diff --git a/impl/maven-core/src/test/java/org/apache/maven/project/DefaultProjectRealmCacheTest.java b/impl/maven-core/src/test/java/org/apache/maven/project/DefaultProjectRealmCacheTest.java new file mode 100644 index 000000000000..d500818e42d5 --- /dev/null +++ b/impl/maven-core/src/test/java/org/apache/maven/project/DefaultProjectRealmCacheTest.java @@ -0,0 +1,73 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.project; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; + +import org.codehaus.plexus.classworlds.realm.ClassRealm; +import org.eclipse.aether.graph.DependencyFilter; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.mockito.Mockito.mock; + +class DefaultProjectRealmCacheTest { + + @Test + void testConcurrentPutWithSameKey() throws Exception { + DefaultProjectRealmCache cache = new DefaultProjectRealmCache(); + ClassRealm realm = mock(ClassRealm.class); + DependencyFilter filter = mock(DependencyFilter.class); + ProjectRealmCache.Key key = cache.createKey(List.of(realm)); + + int threadCount = 10; + CyclicBarrier barrier = new CyclicBarrier(threadCount); + ExecutorService executor = Executors.newFixedThreadPool(threadCount); + List> futures = new ArrayList<>(); + + for (int i = 0; i < threadCount; i++) { + futures.add(executor.submit(() -> { + barrier.await(); + try { + cache.put(key, mock(ClassRealm.class), mock(DependencyFilter.class)); + return true; + } catch (IllegalStateException e) { + return false; + } + })); + } + + int successCount = 0; + for (Future f : futures) { + if (f.get()) { + successCount++; + } + } + executor.shutdown(); + + assertEquals(1, successCount, "Only one put should succeed"); + assertNotNull(cache.get(key)); + } +} From df6375402e25cbf503f28c219e4a63b6257fe73b Mon Sep 17 00:00:00 2001 From: elharo Date: Thu, 30 Jul 2026 13:27:22 +0000 Subject: [PATCH 2/2] Fix checkstyle: remove unused variable and import in DefaultProjectRealmCacheTest --- .../maven/project/DefaultProjectRealmCacheTest.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/impl/maven-core/src/test/java/org/apache/maven/project/DefaultProjectRealmCacheTest.java b/impl/maven-core/src/test/java/org/apache/maven/project/DefaultProjectRealmCacheTest.java index d500818e42d5..ab4fc5efd805 100644 --- a/impl/maven-core/src/test/java/org/apache/maven/project/DefaultProjectRealmCacheTest.java +++ b/impl/maven-core/src/test/java/org/apache/maven/project/DefaultProjectRealmCacheTest.java @@ -26,7 +26,6 @@ import java.util.concurrent.Future; import org.codehaus.plexus.classworlds.realm.ClassRealm; -import org.eclipse.aether.graph.DependencyFilter; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -39,7 +38,6 @@ class DefaultProjectRealmCacheTest { void testConcurrentPutWithSameKey() throws Exception { DefaultProjectRealmCache cache = new DefaultProjectRealmCache(); ClassRealm realm = mock(ClassRealm.class); - DependencyFilter filter = mock(DependencyFilter.class); ProjectRealmCache.Key key = cache.createKey(List.of(realm)); int threadCount = 10; @@ -53,15 +51,15 @@ void testConcurrentPutWithSameKey() throws Exception { try { cache.put(key, mock(ClassRealm.class), mock(DependencyFilter.class)); return true; - } catch (IllegalStateException e) { + } catch (IllegalStateException ex) { return false; } })); } int successCount = 0; - for (Future f : futures) { - if (f.get()) { + for (Future futre : futures) { + if (futre.get()) { successCount++; } }