gnodet-bot commented on code in PR #2217:
URL: https://github.com/apache/maven-resolver/pull/2217#discussion_r4329323609
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/filter/ruletree/Node.java:
##########
@@ -18,36 +18,32 @@
*/
package org.eclipse.aether.internal.impl.filter.ruletree;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.HashMap;
/**
* A tree structure with rules.
*/
abstract class Node<N extends Node<?>> {
protected final String name;
- protected final ConcurrentHashMap<String, N> siblings;
+ protected final HashMap<String, N> children;
Review Comment:
⚠️ **Thread-safety regression**: switching from `ConcurrentHashMap` to
`HashMap` breaks concurrent access in `GroupTree`.
`GroupIdRemoteRepositoryFilterSource.postProcess()` calls `.compute(normalized,
...).loadNode(line)` — the `.loadNode()` runs outside the atomic `compute()`,
so multiple threads resolving artifacts from the same repository mutate the
same `GroupTree.children` concurrently. `HashMap` is not thread-safe here.
Either keep `ConcurrentHashMap` for this field, or synchronize
`GroupTree.loadNode()`.
(`PrefixTree` is unaffected — its tree is always fully built inside
`computeIfAbsent` before being published.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
