Improve thread safety across modules by gnodet · Pull Request #1902 · apache/maven-resolver

gnodet · 2026-06-07T07:37:04Z

Thread Safety Improvements

This PR addresses 31 thread safety issues identified through a systematic audit of the codebase. Each fix is a separate commit for easy review.

HIGH severity fixes (6)

IpcClient close/reconnect lifecycle — reset initialized flag in close() to allow proper reconnection
IpcClient field visibility — make socket/output/input volatile for cross-thread visibility
IpcServer clients HashMap — use ConcurrentHashMap instead of plain HashMap
IpcNamedLock context leak — send unlock on lock timeout to prevent server-side context leak
BasicAuthCache — use ConcurrentHashMap-backed AuthCache to prevent corruption with preemptive auth
SmartExecutor RejectedExecutionException — catch REE to prevent await() hanging forever

MEDIUM severity fixes (15)

DataPool cache initialization — use computeIfAbsent instead of get-then-put race
WeakInternPool.intern() — use compute() for atomic check-and-update
Results.addException/addCycle — synchronize for parallelStream safety
DeferredCredentialsProvider — extend synchronized block to cover delegate read
WagonTransporter close-while-executing — track in-flight wagons for proper cleanup
DefaultProxySelector — use CopyOnWriteArrayList
DefaultMirrorSelector — use CopyOnWriteArrayList
DefaultAuthenticationSelector — use ConcurrentHashMap
SimpleArtifactTypeRegistry — use ConcurrentHashMap
CachingArtifactTypeRegistry — use ConcurrentHashMap.computeIfAbsent()
IpcServer.expirationCheck() — guard against negative Thread.sleep() argument
IpcServer Lock memory leak — remove empty locks from map after unlock
IpcServer Lock.unlock() — complete futures outside monitor to avoid I/O under lock
NamedLockFactorySupport getLock/shutdown — use ReadWriteLock to prevent race
Redisson trySetPermits() — log warning when return value is false

LOW severity fixes (10)

GenericVersionScheme — fix racy cache statistics
Results.errorPath — add volatile
BasicRepositoryConnectorFactory.priority — add volatile
TransferResource.contentLength/resumeOffset — add volatile
Lazy hashCode fields — add volatile in 4 selector/traverser classes
ChainedWorkspaceReader.repository — add volatile
CompositeNamedLock — remove redundant double unlockAll()
WagonTransporter pollWagon — release wagon on reconnect failure
MinioTransporter implPut — close InputStream properly
GnupgSignatureArtifactGenerator — use CopyOnWriteArrayList

Not addressed (4)

F-05 (IpcServer phantom lock on partial multi-key acquisition) — complex architectural change; deferred
F-23 (WebDAV detection redundant OPTIONS) — benign redundancy, accepted
F-28 (DefaultDependencyNode children aliasing) — risk of functional regression
F-34 (File transport TOCTOU) — known limitation of filesystem operations

F-01: close(Throwable) now sets initialized=false before nulling socket/input/output fields, so ensureInitialized() will properly reinitialize on the next send() attempt after a close. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-02: Declare socket, output, input, and receiver fields as volatile so that writes in close(Throwable) are visible to concurrent readers in send() and receive() without requiring synchronization on this. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-03: Replace plain HashMap with ConcurrentHashMap for the clients map so that close() and expirationCheck() can safely access it without synchronization, preventing ConcurrentModificationException and data races. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-04: When client.lock() throws TimeoutException, send an unlock request to clean up the server-side context created by newContext(). Previously the context would leak in IpcServer.contexts indefinitely. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…d AuthCache F-06: Replace BasicAuthCache (backed by plain HashMap) with a ConcurrentAuthCache backed by ConcurrentHashMap, preventing data corruption when concurrent requests call authCache.put() during preemptive auth. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-07: When executor.submit() throws RejectedExecutionException, fall back to running the task on the caller thread. This prevents RunnableErrorForwarder.await() from hanging forever when the counter was incremented by wrap() but the task never runs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-08: Replace get-then-put pattern with cache.computeIfAbsent() for all four intern pools, preventing concurrent DataPool constructors from creating and using detached pool instances. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-09: Use map.compute() instead of separate get/put to make the interning operation atomic, preventing duplicate objects from defeating heap deduplication during concurrent descriptor resolution. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-10: Synchronize addException() and addCycle() in Results to prevent data races when called concurrently from BfDependencyCollector's parallelStream during version range resolution. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-11: Move delegate.getCredentials() inside the synchronized(factories) block to prevent concurrent read/write on the underlying HashMap-backed BasicCredentialsProvider. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-12: After transfer completes, check if the transporter was closed during execution. If so, disconnect and release the wagon instead of returning it to the queue where it would never be cleaned up. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-13: Replace plain ArrayList with CopyOnWriteArrayList to prevent ConcurrentModificationException when add() is called concurrently with getProxy() iteration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-14: Replace plain ArrayList with CopyOnWriteArrayList to prevent ConcurrentModificationException when add() is called concurrently with getMirror()/findMirror() iteration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-15: Replace plain HashMap with ConcurrentHashMap to prevent data corruption when add() and getAuthentication() are called from different threads. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-16: Replace plain HashMap with ConcurrentHashMap to ensure visibility of types populated on the main thread to worker threads querying via the session. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-17: Replace plain HashMap with ConcurrentHashMap to prevent corruption when concurrent parallel stream workers query the artifact type registry simultaneously. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-18: Clamp Thread.sleep() argument to minimum 1ms to prevent IllegalArgumentException when left goes negative while clients are still connected. Previously this silently killed the expiration thread, causing the server to never shut down. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-19: Remove Lock entries from the locks ConcurrentHashMap when holders and waiters are both empty after unlock, preventing unbounded accumulation over the server's lifetime. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-20: Collect futures to complete during the synchronized block but complete them after releasing the Lock monitor, preventing I/O operations (socket writes in thenRun callbacks) from being serialized under the lock. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-21: Re-check shutdown flag inside the locks.compute() lambda to prevent creating locks on a backend that was shut down between the initial check and the actual lock creation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-22: Check the return value of trySetPermits(). If it returns false (e.g., a crashed process left the semaphore in a depleted state), log a warning instead of silently proceeding with a potentially broken semaphore. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-24: Use a single computeIfAbsent() call instead of separate get() then computeIfAbsent(), and derive hit/miss statistics from whether the mapping function was invoked. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-25: Make errorPath volatile to ensure cross-thread visibility when set from parallel stream workers and read from the main thread. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-26: Make the priority field volatile to ensure cross-thread visibility between setPriority() and getPriority() calls. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-27: Make contentLength and resumeOffset volatile to ensure visibility when written on worker threads and read by monitoring or logging threads. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-29: Make cached hashCode fields volatile in ExclusionDependencySelector, AndDependencySelector, AndDependencyTraverser, and ChainedVersionFilter to ensure visibility across threads. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-30: Make the repository field volatile to ensure cross-thread visibility between construction/lazy update and concurrent readers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-31: Return false immediately after unlockAll() on lock failure instead of breaking out of the loop and calling unlockAll() again on the already-empty deque. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…nstead of re-queuing

F-33: Wrap task.newInputStream() in try-with-resources so the stream is properly closed after Files.copy(), preventing resource leaks on both normal and error paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

F-35: Replace plain ArrayList with CopyOnWriteArrayList for signatureTempFiles as a defensive measure against potential concurrent access between generate() and close(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

gnodet · 2026-06-07T09:16:37Z

Remaining Findings Analysis (F-05, F-23, F-28, F-34)

The 4 findings not addressed in this PR were analyzed in detail against the actual source code:

Finding	Severity	Verdict	Rationale
F-05	HIGH	SKIP — False positive	`Lock.unlock()` dual-path logic (holders + waiters removal) under a single monitor already prevents phantom locks. The `thenRun` callback only writes to sockets — it cannot re-add a context to any holders list.
F-23	MEDIUM	SKIP — Benign race	Redundant idempotent OPTIONS requests at worst. The `volatile Boolean` ensures visibility. Synchronizing around an HTTP round-trip would be worse — serializing concurrent PUTs behind network I/O.
F-28	LOW	DEFER	Valid list aliasing through DataPool cache, but intentional for memory optimization. A 2-line defensive copy fix exists if wanted, but no known bug reports and existing transformers already partially defend against it.
F-34	LOW	SKIP — Not a bug	Universal filesystem TOCTOU inherent to all Java NIO. Same `createDirectories` + `newOutputStream` pattern used in 4+ places in the codebase. Standard idiomatic Java.

Full analysis: see resolver-remaining-findings-analysis.md in the audit artifacts.

…le overloads Pooled.submit(Callable) and Limited.submit(Callable) did not catch RejectedExecutionException, unlike their Runnable counterparts. On rejection, Pooled would let the REE propagate with a never-completed future, and Limited would additionally leak a semaphore permit. Apply the same try-catch-fallback pattern: on rejection, run the callable on the caller thread and complete the future inline. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The unlock() path had a race: after checking l.isEmpty(), another thread could computeIfAbsent and re-use the same Lock object, then the remove(l.key, l) would remove a Lock that now has holders. Replace the two-step isEmpty() check + remove() with a single locks.compute() call that atomically checks emptiness and removes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Collections.synchronizedMap does not make computeIfAbsent atomic; the default Map.computeIfAbsent implementation performs a non-atomic get-then-put sequence, risking concurrent structural modification of the underlying WeakHashMap. Wrap the computeIfAbsent call in synchronized(versionCache) to use the same monitor that synchronizedMap uses internally. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Collections.synchronizedMap does not make compute() atomic; the default Map.compute implementation performs a non-atomic read-modify sequence, risking concurrent structural modification of the underlying WeakHashMap. Wrap the compute() call in synchronized(map) to use the same monitor that synchronizedMap uses internally. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The write paths addException() and addCycle() are synchronized but getResult() and getErrorPath() were not, so readers could observe stale data without a happens-before edge from the writer. Make both getter methods synchronized to establish proper visibility guarantees for cross-thread reads. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…tifacts The signatureTempFiles field was changed to CopyOnWriteArrayList for thread safety, but the artifacts field (also mutated via addAll in generate()) was left as a plain ArrayList. Change artifacts to CopyOnWriteArrayList for consistency, since both fields are mutated and iterated in the same methods. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR targets a broad set of concurrency and thread-safety fixes across Maven Resolver modules, primarily by improving cross-thread visibility, replacing non-thread-safe collections, and hardening resource lifecycle handling under concurrent use.

Changes:

Replaced several shared mutable collections with concurrent alternatives (e.g., ConcurrentHashMap, CopyOnWriteArrayList) and tightened atomic cache initialization patterns.
Improved IPC named-lock server/client behavior (lock cleanup, lifecycle reset, avoiding completing futures under monitors).
Hardened transport/executor behavior under shutdown/rejection scenarios and improved resource handling (e.g., ensuring streams are closed).

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
maven-resolver-util/src/main/java/org/eclipse/aether/util/version/GenericVersionScheme.java	Makes version cache hit/miss accounting atomic with cache population.
maven-resolver-util/src/main/java/org/eclipse/aether/util/repository/DefaultProxySelector.java	Uses a thread-safe list for proxy definitions.
maven-resolver-util/src/main/java/org/eclipse/aether/util/repository/DefaultMirrorSelector.java	Uses a thread-safe list for mirror definitions.
maven-resolver-util/src/main/java/org/eclipse/aether/util/repository/DefaultAuthenticationSelector.java	Uses a concurrent map for repository authentication lookup.
maven-resolver-util/src/main/java/org/eclipse/aether/util/repository/ChainedWorkspaceReader.java	Adds `volatile` for cross-thread visibility of repository reference.
maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/version/ChainedVersionFilter.java	Makes lazy `hashCode` cache safely publishable across threads.
maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/traverser/AndDependencyTraverser.java	Makes lazy `hashCode` cache safely publishable across threads.
maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/selector/ExclusionDependencySelector.java	Makes lazy `hashCode` cache safely publishable across threads.
maven-resolver-util/src/main/java/org/eclipse/aether/util/graph/selector/AndDependencySelector.java	Makes lazy `hashCode` cache safely publishable across threads.
maven-resolver-util/src/main/java/org/eclipse/aether/util/concurrency/SmartExecutor.java	Adds rejection handling to avoid hangs when submission is refused.
maven-resolver-util/src/main/java/org/eclipse/aether/util/artifact/SimpleArtifactTypeRegistry.java	Uses a concurrent map for artifact type registry storage.
maven-resolver-transport-wagon/src/main/java/org/eclipse/aether/transport/wagon/WagonTransporter.java	Improves wagon lifecycle handling on reconnect failures / transporter close.
maven-resolver-transport-minio/src/main/java/org/eclipse/aether/transport/minio/MinioTransporter.java	Ensures `InputStream` is closed when uploading from an input stream.
maven-resolver-transport-apache/src/main/java/org/eclipse/aether/transport/apache/DeferredCredentialsProvider.java	Extends synchronization to cover delegate read for credential lookup.
maven-resolver-transport-apache/src/main/java/org/eclipse/aether/transport/apache/ApacheTransporter.java	Introduces a concurrent `AuthCache` implementation and uses it for session caching.
maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/NamedLockFactorySupport.java	Rejects lock creation after shutdown during map compute to prevent races.
maven-resolver-named-locks/src/main/java/org/eclipse/aether/named/support/CompositeNamedLock.java	Removes redundant unlock path on partial acquisition failures.
maven-resolver-named-locks-redisson/src/main/java/org/eclipse/aether/named/redisson/RedissonSemaphoreNamedLockFactory.java	Logs a warning when `trySetPermits()` fails.
maven-resolver-named-locks-ipc/src/main/java/org/eclipse/aether/named/ipc/IpcServer.java	Uses concurrent client map; avoids completing futures under lock; cleans up empty locks; guards sleep duration.
maven-resolver-named-locks-ipc/src/main/java/org/eclipse/aether/named/ipc/IpcNamedLock.java	Unlocks context on lock timeout to prevent server-side context leaks.
maven-resolver-named-locks-ipc/src/main/java/org/eclipse/aether/named/ipc/IpcClient.java	Improves cross-thread visibility and resets initialization on close for reconnect.
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DependencyCollectorDelegate.java	Synchronizes `Results` mutation/access for parallel collection safety.
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/DataPool.java	Removes get-then-put races via `computeIfAbsent`; makes weak interning atomic.
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/collect/CachingArtifactTypeRegistry.java	Switches internal cache to `ConcurrentHashMap` and refines cache get flow.
maven-resolver-generator-gnupg/src/main/java/org/eclipse/aether/generator/gnupg/GnupgSignatureArtifactGenerator.java	Uses copy-on-write lists for thread-safe artifact/temp-file tracking.
maven-resolver-connector-basic/src/main/java/org/eclipse/aether/connector/basic/BasicRepositoryConnectorFactory.java	Makes `priority` safely publishable across threads via `volatile`.
maven-resolver-api/src/main/java/org/eclipse/aether/transfer/TransferResource.java	Adds `volatile` to mutable transfer state for cross-thread visibility.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

gnodet · 2026-06-08T08:57:31Z

+            } catch (RejectedExecutionException e) {
+                runnable.run();
+            }


Fixed. The fallback runnable.run() is now wrapped in a try-catch(RuntimeException | Error) that swallows the exception. This preserves the async submit() semantics where exceptions are captured by the Future. Callers like RunnableErrorForwarder already record the error before re-throwing, so it will still be reported via await().

gnodet · 2026-06-08T08:57:34Z

+                } catch (RejectedExecutionException e) {
                    try {
                        runnable.run();
                    } finally {
                        semaphore.release();


Fixed. Same approach as the Pooled case above — the fallback runnable.run() in Limited.submit(Runnable) now catches and swallows RuntimeException | Error to match async submission semantics. The semaphore is still properly released in the finally block.

gnodet · 2026-06-08T08:57:37Z

    public ArtifactType get(String typeId) {
        ArtifactType type = types.get(typeId);
-
-        if (type == null) {
-            type = delegate.get(typeId);
+        if (type != null) {
+            return type;
+        }


Fixed. Replaced the get-then-delegate-then-put sequence with types.computeIfAbsent(typeId, delegate::get) for atomic cache population. This avoids redundant delegate.get() calls under contention.

gnodet · 2026-06-08T08:57:39Z

+        this.artifacts = new CopyOnWriteArrayList<>(artifacts);
        this.signableArtifactPredicate = signableArtifactPredicate;
        this.secretKey = secretKey;
        this.privateKey = privateKey;
        this.hashSubPackets = hashSubPackets;
        this.keyInfo = keyInfo;
-        this.signatureTempFiles = new ArrayList<>();
+        this.signatureTempFiles = new CopyOnWriteArrayList<>();
        logger.debug("Created generator using key {}", keyInfo);


Fixed. Replaced CopyOnWriteArrayList with plain ArrayList for both artifacts and signatureTempFiles, and made generate() and close() synchronized. This avoids the array-copy overhead on each write while still providing thread safety and happens-before guarantees between the two methods.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

cstamas · 2026-06-08T12:19:23Z

    private final RequestTrace trace;

-    private long contentLength = -1L;
+    private volatile long contentLength = -1L;


cstamas · 2026-06-08T12:20:20Z

    private final Map<String, ProvidedChecksumsSource> providedChecksumsSources;

-    private float priority;
+    private volatile float priority;


cstamas · 2026-06-08T12:21:43Z


    @Override
-    public Collection<? extends Artifact> generate(Collection<? extends Artifact> generatedArtifacts) {
+    public synchronized Collection<? extends Artifact> generate(Collection<? extends Artifact> generatedArtifacts) {


cstamas · 2026-06-08T12:26:31Z

-                    return pooled;
-                }
+            Object[] result = new Object[1];
+            synchronized (map) {


cstamas · 2026-06-08T12:26:52Z

        final int maxCycles;

-        String errorPath;
+        volatile String errorPath;


cstamas · 2026-06-08T12:30:56Z

    private final Set<? extends DependencySelector> selectors;

-    private int hashCode;
+    private volatile int hashCode;


The hashCode() is not concurrency safe here, so I'd make this final instead and modify hashCode() to just return the final int hashCode instead.

cstamas · 2026-06-08T12:30:59Z

    private final Exclusion[] exclusions;

-    private int hashCode;
+    private volatile int hashCode;


The hashCode() is not concurrency safe here, so I'd make this final instead and modify hashCode() to just return the final int hashCode instead.

cstamas · 2026-06-08T12:31:15Z

    private final Set<? extends DependencyTraverser> traversers;

-    private int hashCode;
+    private volatile int hashCode;


The hashCode() is not concurrency safe here, so I'd make this final instead and modify hashCode() to just return the final int hashCode instead.

cstamas · 2026-06-08T12:31:31Z

    private final VersionFilter[] filters;

-    private int hashCode;
+    private volatile int hashCode;


The hashCode() is not concurrency safe here, so I'd make this final instead and modify hashCode() to just return the final int hashCode instead.

cstamas · 2026-06-08T12:34:53Z

    private final List<WorkspaceReader> readers = new ArrayList<>();

-    private WorkspaceRepository repository;
+    private volatile WorkspaceRepository repository;


Isn't this rather final?

cstamas · 2026-06-08T12:36:55Z

Overall looks good, but there are IMHO some unnecessary usage of volatile (not biggie, but anyone seeing this keyword may frantically look for "where is this concurrently used?" and maybe find nothing), and similarly for synchronized. Latter even slows down things in case of true single threaded run, no? (ie obtaining object monitor...)

gnodet and others added 30 commits June 7, 2026 06:18

Fix DefaultProxySelector — use CopyOnWriteArrayList for proxies

122c852

F-13: Replace plain ArrayList with CopyOnWriteArrayList to prevent ConcurrentModificationException when add() is called concurrently with getProxy() iteration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Fix DefaultAuthenticationSelector — use ConcurrentHashMap

290a392

F-15: Replace plain HashMap with ConcurrentHashMap to prevent data corruption when add() and getAuthentication() are called from different threads. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Fix SimpleArtifactTypeRegistry — use ConcurrentHashMap

5c09c9e

F-16: Replace plain HashMap with ConcurrentHashMap to ensure visibility of types populated on the main thread to worker threads querying via the session. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Fix CachingArtifactTypeRegistry — use ConcurrentHashMap

49ff114

F-17: Replace plain HashMap with ConcurrentHashMap to prevent corruption when concurrent parallel stream workers query the artifact type registry simultaneously. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Fix NamedLockFactorySupport getLock/shutdown race

2e2522e

F-21: Re-check shutdown flag inside the locks.compute() lambda to prevent creating locks on a backend that was shut down between the initial check and the actual lock creation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Fix GenericVersionScheme racy cache statistics

3f59c88

F-24: Use a single computeIfAbsent() call instead of separate get() then computeIfAbsent(), and derive hit/miss statistics from whether the mapping function was invoked. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Fix Results.errorPath visibility — add volatile

ca22326

F-25: Make errorPath volatile to ensure cross-thread visibility when set from parallel stream workers and read from the main thread. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Fix BasicRepositoryConnectorFactory.priority — add volatile

fee4cfb

F-26: Make the priority field volatile to ensure cross-thread visibility between setPriority() and getPriority() calls. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Fix TransferResource field visibility — add volatile

7d8dc3a

F-27: Make contentLength and resumeOffset volatile to ensure visibility when written on worker threads and read by monitoring or logging threads. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Fix ChainedWorkspaceReader.repository — add volatile

067e6fb

F-30: Make the repository field volatile to ensure cross-thread visibility between construction/lazy update and concurrent readers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Fix CompositeNamedLock — remove redundant double unlockAll

79c11d5

F-31: Return false immediately after unlockAll() on lock failure instead of breaking out of the loop and calling unlockAll() again on the already-empty deque. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Fix WagonTransporter pollWagon — release wagon on reconnect failure i…

a8c68f2

…nstead of re-queuing

Fix MinioTransporter implPut — close InputStream from task

6fce413

F-33: Wrap task.newInputStream() in try-with-resources so the stream is properly closed after Files.copy(), preventing resource leaks on both normal and error paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

gnodet and others added 6 commits June 7, 2026 09:43

cstamas added this to the 2.0.19 milestone Jun 7, 2026

cstamas added the enhancement New feature or request label Jun 7, 2026

gnodet marked this pull request as ready for review June 7, 2026 12:19

slachiewicz requested a review from Copilot June 7, 2026 18:23

slachiewicz approved these changes Jun 7, 2026

View reviewed changes

Copilot started reviewing on behalf of slachiewicz June 7, 2026 18:24 View session

Copilot AI reviewed Jun 7, 2026

View reviewed changes

gnodet and others added 2 commits June 8, 2026 08:56

Address Copilot review comments on thread safety

a791e56

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Fix code formatting (spotless)

8ecda11

cstamas reviewed Jun 8, 2026

View reviewed changes

Conversation

gnodet commented Jun 7, 2026

Thread Safety Improvements

HIGH severity fixes (6)

MEDIUM severity fixes (15)

LOW severity fixes (10)

Not addressed (4)

Uh oh!

gnodet commented Jun 7, 2026

Remaining Findings Analysis (F-05, F-23, F-28, F-34)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cstamas commented Jun 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants