From 9f7446b6648c61c05a224b4e968c5125de1f8316 Mon Sep 17 00:00:00 2001 From: Multi-Agent Maintainer Date: Mon, 23 Feb 2026 17:16:22 +0000 Subject: [PATCH] Fix thread-safety issue in DefaultModelValidator (#11618) The DefaultModelValidator class uses a shared HashSet (validIds) as an instance field while being a @Singleton. When multiple threads validate models concurrently (e.g., using breadth-first dependency collector with `aether.dependencyCollector.impl=bf`), concurrent read/write operations on the HashSet cause ClassCastException: java.lang.ClassCastException: class java.util.HashMap$Node cannot be cast to class java.util.HashMap$TreeNode This occurs when the HashMap underlying the HashSet resizes and tree-ifies buckets while another thread is reading. The fix aligns with the Maven 4 implementation (maven-impl module) which already uses ConcurrentHashMap.newKeySet() for thread-safety. Changes: - Replace HashSet with ConcurrentHashMap.newKeySet() for validIds - Add concurrent validation test to prevent regression Fixes #11618 --- .../validation/DefaultModelValidator.java | 5 +- .../validation/DefaultModelValidatorTest.java | 57 +++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/compat/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java b/compat/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java index 4bde6c49989e..b14a85a02381 100644 --- a/compat/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java +++ b/compat/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java @@ -34,6 +34,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.Consumer; import java.util.function.Supplier; import java.util.regex.Matcher; @@ -89,7 +90,9 @@ public class DefaultModelValidator implements ModelValidator { private static final String EMPTY = ""; - private final Set validIds = new HashSet<>(); + // Thread-safe set required because class is @Singleton and validIds is accessed concurrently + // See: https://github.com/apache/maven/issues/11618 + private final Set validIds = ConcurrentHashMap.newKeySet(); private ModelVersionProcessor versionProcessor; diff --git a/compat/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java b/compat/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java index a5f5d8548cb7..eccf1ae3084f 100644 --- a/compat/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java +++ b/compat/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java @@ -902,4 +902,61 @@ void profileActivationPropertyWithProjectExpression() throws Exception { + "${project.version} expressions are not supported during profile activation.", result.getWarnings().get(1)); } + + /** + * Validates thread-safety of DefaultModelValidator during concurrent model validation. + * + *

This test addresses GitHub issue #11618 where concurrent access to a shared + * {@code HashSet} in {@code DefaultModelValidator} could cause {@code ClassCastException}. + * The underlying issue occurs when multiple threads access a non-thread-safe {@code HashSet} + * (backed by {@code HashMap}) during internal restructuring operations. + * + *

The fix replaces {@code HashSet} with {@code ConcurrentHashMap.newKeySet()} to provide + * thread-safe concurrent access without external synchronization. + * + * @see GitHub #11618 + */ + @Test + void testConcurrentValidation() throws Exception { + int threadCount = 10; + int iterationsPerThread = 100; + java.util.concurrent.CountDownLatch startLatch = new java.util.concurrent.CountDownLatch(1); + java.util.concurrent.CountDownLatch doneLatch = new java.util.concurrent.CountDownLatch(threadCount); + java.util.concurrent.atomic.AtomicReference failure = new java.util.concurrent.atomic.AtomicReference<>(); + + // Create multiple threads that will validate models concurrently + for (int t = 0; t < threadCount; t++) { + final int threadId = t; + new Thread(() -> { + try { + startLatch.await(); // Wait for all threads to be ready + for (int i = 0; i < iterationsPerThread; i++) { + Model model = new Model(); + model.setModelVersion("4.0.0"); + model.setGroupId("test.group" + threadId); + model.setArtifactId("test-artifact-" + threadId + "-" + i); + model.setVersion("1.0.0"); + + SimpleProblemCollector problems = new SimpleProblemCollector(model); + validator.validateEffectiveModel(model, new DefaultModelBuildingRequest(), problems); + } + } catch (Throwable e) { + failure.compareAndSet(null, e); + } finally { + doneLatch.countDown(); + } + }).start(); + } + + // Start all threads simultaneously + startLatch.countDown(); + + // Wait for all threads to complete + assertTrue(doneLatch.await(30, java.util.concurrent.TimeUnit.SECONDS), "Threads did not complete in time"); + + // Check if any thread encountered an error + if (failure.get() != null) { + throw new AssertionError("Concurrent validation failed: " + failure.get().getMessage(), failure.get()); + } + } }