gnodet commented on code in PR #1902:
URL: https://github.com/apache/maven-resolver/pull/1902#discussion_r3419228678


##########
maven-resolver-generator-gnupg/src/main/java/org/eclipse/aether/generator/gnupg/GnupgSignatureArtifactGenerator.java:
##########
@@ -79,7 +80,7 @@ public String generatorId() {
     }
 
     @Override
-    public Collection<? extends Artifact> generate(Collection<? extends 
Artifact> generatedArtifacts) {
+    public synchronized Collection<? extends Artifact> generate(Collection<? 
extends Artifact> generatedArtifacts) {

Review Comment:
   `generate()` mutates `this.artifacts` (via `addAll`) and 
`this.signatureTempFiles`. It can be called multiple times during an upload 
(once per batch of generated artifacts). If two threads invoke it concurrently, 
the unsynchronized `ArrayList` operations would cause data races. The 
`synchronized` ensures mutual exclusion between `generate()` and `close()` 
which also accesses `signatureTempFiles`.



##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DataPool.java:
##########
@@ -582,16 +563,23 @@ public V get(K key) {
         }
 
         @Override
+        @SuppressWarnings("unchecked")
         public V intern(K key, V value) {
-            WeakReference<V> pooledRef = map.get(key);
-            if (pooledRef != null) {
-                V pooled = pooledRef.get();
-                if (pooled != null) {
-                    return pooled;
-                }
+            Object[] result = new Object[1];
+            synchronized (map) {

Review Comment:
   The original `intern()` had a classic check-then-act race: thread A reads 
`map.get(key)` and finds null/expired, thread B does the same, then both put a 
new entry — defeating the purpose of interning (callers get different instances 
for the same key). Wrapping the check-and-put in `synchronized` + 
`map.compute()` makes the operation atomic. The `synchronized` block is needed 
because `WeakHashMap` is not thread-safe even for single operations.



-- 
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]

Reply via email to