[Backport maven-3.10.x] Fix thread-safety in DefaultModelValidator (fix #11618)

[arturobernalg]

Backport of #11734 to maven-3.10.x.

DefaultModelValidator is a singleton, but its validIds field is a plain HashSet on this branch, which several threads can mutate during model validation, for instance with the breadth-first dependency collector. A concurrent restructuring of the HashMap behind it can make a reader fail with a ClassCastException (#11618). The field is now ConcurrentHashMap.newKeySet(), as on master and on maven-4.0.x, and a null id is no longer looked up in it.

The main change applied after adapting the module path. The test testConcurrentValidation was added by hand to the existing test class. It is a regression guard, and like the original it does not fail reliably without the change, because the race is timing dependent. maven-model-builder and its dependencies pass (207 tests) and spotless is clean. I did not run the integration tests.

---

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
check one of the following:

• I hereby declare this contribution to be licensed under the Apache License Version 2.0, January 2004
• I hereby declare that I have filed an Apache Individual Contributor License Agreement.

[gnodet-bot]

Clean backport. The two-line production change is exactly right: ConcurrentHashMap.newKeySet() fixes the thread-safety race, and the id != null guard is a required companion because ConcurrentHashMap.contains(null) throws NullPointerException (unlike HashSet). Without it, a missing groupId or artifactId would NPE at the cache lookup before reaching the null-handling in validateStringNotEmpty.

One note on the concurrency test: the test parameters (10 threads × 100 iterations) are too small to reliably trigger the HashMap tree-restructuring race that causes the original ClassCastException. I confirmed this locally — the test passes identically with the pre-fix HashSet. This is an inherent limitation of deterministic concurrency tests; the race requires specific thread interleaving during resize. The test still has value as a smoke test (no deadlock, no unexpected exceptions), but it cannot dynamically validate the fix. Consider bumping to at least 50 threads × 1000 iterations to meaningfully increase the probability of catching a future regression.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[arturobernalg]

Raised the test to 50 threads and 1000 iterations in 96c661f, as suggested. I checked it against the old HashSet: with the old code it failed in 1 of 3 runs, while the previous size never failed in 3 runs. It is still timing dependent and cannot fail every time, but it catches the race more often. With the fix it passed in 3 of 3 runs.

[cstamas]

@arturobernalg you aware of #13303 ?

[cstamas]

@arturobernalg merged "by age", but thanks!