From 6643019fe3b341f47db8a45c6447ed87ca612a8b Mon Sep 17 00:00:00 2001
From: Ryan Schmitt <rschmitt@apache.org>
Date: Wed, 14 Jan 2026 21:08:56 -0800
Subject: [PATCH] ComplexCancellable: Fix race condition
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

I discovered a race condition while investigating a sporadic test
failure in the client:

```
[ERROR]   HttpMinimalIntegrationTests$H2Tls>AbstractHttpAsyncFundamentalsTest.testSequentialGetRequests:86 » Execution org.apache.hc.core5.http2.H2StreamResetException: Cancelled
```

I discovered that two threads are somehow racing when calling
`ComplexCancellable.setDependency()`:

```
setDependency dep=org.apache.hc.client5.http.impl.async.MinimalH2AsyncClient$$Lambda$655/251536271@633478cb ct=ForkJoinPool-1-worker-2
setDependency dep=[id=1, reserved=false, removeClosed=false, localClosed=false, localReset=false] ct=httpclient-dispatch-1
setDependency dep=org.apache.hc.client5.http.impl.async.MinimalH2AsyncClient$$Lambda$655/251536271@2e66671a ct=ForkJoinPool-1-worker-2
setDependency dep=[id=3, reserved=false, removeClosed=false, localClosed=false, localReset=false] ct=httpclient-dispatch-1
setDependency dep=org.apache.hc.client5.http.impl.async.MinimalH2AsyncClient$$Lambda$655/251536271@32f4e96 ct=ForkJoinPool-1-worker-2
setDependency dep=[id=5, reserved=false, removeClosed=false, localClosed=false, localReset=false] ct=httpclient-dispatch-1
```

The implementation of `setDependency` was performing a single CAS and
interpreting failure to mean that the mark bit had been set -- in other
words, that another thread had called `cancel()`, rather than
`setDependency()`.

This change corrects the implementation of `setDependency` by having it
run the CAS in a loop. On each update failure, it checks if the mark bit
has been set (nothing ever unsets it). If so, it calls
`dependency.cancel()` and returns; else, it just tries again.
---
 .../core5/concurrent/ComplexCancellable.java  |  8 +++---
 .../concurrent/TestComplexCancellable.java    | 27 +++++++++++++++++--
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/httpcore5/src/main/java/org/apache/hc/core5/concurrent/ComplexCancellable.java b/httpcore5/src/main/java/org/apache/hc/core5/concurrent/ComplexCancellable.java
index 1b43219752..eabfc530ad 100644
--- a/httpcore5/src/main/java/org/apache/hc/core5/concurrent/ComplexCancellable.java
+++ b/httpcore5/src/main/java/org/apache/hc/core5/concurrent/ComplexCancellable.java
@@ -53,9 +53,11 @@ public boolean isCancelled() {
     @Override
     public void setDependency(final Cancellable dependency) {
         Args.notNull(dependency, "dependency");
-        final Cancellable actualDependency = dependencyRef.getReference();
-        if (!dependencyRef.compareAndSet(actualDependency, dependency, false, false)) {
-            dependency.cancel();
+        while (!dependencyRef.compareAndSet(dependencyRef.getReference(), dependency, false, false)) {
+            if (dependencyRef.isMarked()) {
+                dependency.cancel();
+                return;
+            }
         }
     }
 
diff --git a/httpcore5/src/test/java/org/apache/hc/core5/concurrent/TestComplexCancellable.java b/httpcore5/src/test/java/org/apache/hc/core5/concurrent/TestComplexCancellable.java
index 42806b9b84..e728bc6450 100644
--- a/httpcore5/src/test/java/org/apache/hc/core5/concurrent/TestComplexCancellable.java
+++ b/httpcore5/src/test/java/org/apache/hc/core5/concurrent/TestComplexCancellable.java
@@ -27,11 +27,13 @@
 package org.apache.hc.core5.concurrent;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 
 import org.hamcrest.CoreMatchers;
-import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
+import java.util.concurrent.CountDownLatch;
+
 class TestComplexCancellable {
 
     @Test
@@ -41,7 +43,7 @@ void testCancelled() {
         final BasicFuture<Object> dependency1 = new BasicFuture<>(null);
         cancellable.setDependency(dependency1);
 
-        Assertions.assertFalse(cancellable.isCancelled());
+        assertFalse(cancellable.isCancelled());
 
         cancellable.cancel();
         assertThat(cancellable.isCancelled(), CoreMatchers.is(true));
@@ -52,4 +54,25 @@ void testCancelled() {
         assertThat(dependency2.isCancelled(), CoreMatchers.is(true));
     }
 
+    @Test
+    void testSetDependencyRace() throws InterruptedException {
+        final ComplexCancellable cancellable = new ComplexCancellable();
+        final BasicFuture<Object> dependency1 = new BasicFuture<>(null);
+        final BasicFuture<Object> dependency2 = new BasicFuture<>(null);
+        final CountDownLatch latch = new CountDownLatch(2);
+
+        for (int i = 0; i < 2; i++) {
+            new Thread(() -> {
+                for (int j = 0; j < 5_000; j++) {
+                    cancellable.setDependency(dependency1);
+                    cancellable.setDependency(dependency2);
+                }
+                latch.countDown();
+            }).start();
+        }
+        latch.await();
+
+        assertFalse(dependency1.isCancelled());
+        assertFalse(dependency2.isCancelled());
+    }
 }