[httpcomponents-core] 01/01: HTTPCORE-588: race condition in ComplexCancellable that can lead to operational dependency not being correctly cancelled

[ol...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

olegk pushed a commit to branch HTTPCORE-588
in repository https://gitbox.apache.org/repos/asf/httpcomponents-core.git

commit 1e9d57e176b142ed05f48928e36c04149c1f9385
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Fri Aug 9 12:22:24 2019 +0200

    HTTPCORE-588: race condition in ComplexCancellable that can lead to operational dependency not being correctly cancelled
---
 .../hc/core5/concurrent/ComplexCancellable.java    | 28 ++++++++++------------
 1 file changed, 13 insertions(+), 15 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 d306d6a..1b43219 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
@@ -26,8 +26,7 @@
  */
 package org.apache.hc.core5.concurrent;
 
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicReference;
+import java.util.concurrent.atomic.AtomicMarkableReference;
 
 import org.apache.hc.core5.util.Args;
 
@@ -40,37 +39,36 @@ import org.apache.hc.core5.util.Args;
  */
 public final class ComplexCancellable implements CancellableDependency {
 
-    private final AtomicReference<Cancellable> dependencyRef;
-    private final AtomicBoolean cancelled;
+    private final AtomicMarkableReference<Cancellable> dependencyRef;
 
     public ComplexCancellable() {
-        this.dependencyRef = new AtomicReference<>(null);
-        this.cancelled = new AtomicBoolean(false);
+        this.dependencyRef = new AtomicMarkableReference<>(null, false);
     }
 
     @Override
     public boolean isCancelled() {
-        return cancelled.get();
+        return dependencyRef.isMarked();
     }
 
     @Override
     public void setDependency(final Cancellable dependency) {
         Args.notNull(dependency, "dependency");
-        if (!cancelled.get()) {
-            dependencyRef.set(dependency);
-        } else {
+        final Cancellable actualDependency = dependencyRef.getReference();
+        if (!dependencyRef.compareAndSet(actualDependency, dependency, false, false)) {
             dependency.cancel();
         }
     }
 
     @Override
     public boolean cancel() {
-        if (cancelled.compareAndSet(false, true)) {
-            final Cancellable dependency = dependencyRef.getAndSet(null);
-            if (dependency != null) {
-                dependency.cancel();
+        while (!dependencyRef.isMarked()) {
+            final Cancellable actualDependency = dependencyRef.getReference();
+            if (dependencyRef.compareAndSet(actualDependency, actualDependency, false, true)) {
+                if (actualDependency != null) {
+                    actualDependency.cancel();
+                }
+                return true;
             }
-            return true;
         }
         return false;
     }