Fix lead transaction cancellation

[mdsal.git] / dom / mdsal-dom-spi / src / main / java / org / opendaylight / mdsal / dom / spi / PingPongTransactionChain.java

diff --git a/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/PingPongTransactionChain.java b/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/PingPongTransactionChain.java
index 8a342fbaa2b3a9c0ad5010cd924d5ac90c23de3b..2f24b0b947c32fd1641fba60429ad58f9b5ddf81 100644 (file)
--- a/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/PingPongTransactionChain.java
+++ b/dom/mdsal-dom-spi/src/main/java/org/opendaylight/mdsal/dom/spi/PingPongTransactionChain.java
@@ -16,7 +16,7 @@ import com.google.common.util.concurrent.FutureCallback;
 import com.google.common.util.concurrent.MoreExecutors;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.VarHandle;
-import java.util.AbstractMap.SimpleImmutableEntry;
+import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Optional;
 import java.util.concurrent.CancellationException;
@@ -342,9 +342,9 @@ public final class PingPongTransactionChain implements DOMTransactionChain {
      *
      * @param tx Backend shared transaction
      * @param frontendTx transaction
-     * @param isOpen indicator whether the transaction was already closed
+     * @return {@code true} if the transaction was cancelled successfully
      */
-    synchronized void cancelTransaction(final PingPongTransaction tx,
+    synchronized boolean cancelTransaction(final PingPongTransaction tx,
             final DOMDataTreeReadWriteTransaction frontendTx) {
         // Attempt to unlock the operation.
         final Object witness = LOCKED_TX.compareAndExchange(this, tx, null);
@@ -356,29 +356,39 @@ public final class PingPongTransactionChain implements DOMTransactionChain {
         if (failed) {
             // The transaction has failed, this is probably the user just clearing up the transaction they had. We have
             // already cancelled the transaction anyway,
-            return;
-        } else if (!backendCancelled) {
-            LOG.warn("Backend transaction cannot be cancelled during cancellation of {}, attempting to continue", tx);
+            return true;
         }
 
-        // We have dealt with canceling the backend transaction and have unlocked the transaction. Since we are still
+        // We have dealt with cancelling the backend transaction and have unlocked the transaction. Since we are still
         // inside the synchronized block, any allocations are blocking on the slow path. Now we have to decide the fate
         // of this transaction chain.
         //
         // If there are no other frontend transactions in this batch we are aligned with backend state and we can
         // continue processing.
         if (frontendTx.equals(tx.getFrontendTransaction())) {
-            LOG.debug("Cancelled transaction {} was head of the batch, resuming processing", tx);
-            return;
+            if (backendCancelled) {
+                LOG.debug("Cancelled transaction {} was head of the batch, resuming processing", tx);
+                return true;
+            }
+
+            // Backend refused to cancel the transaction. Reinstate it to locked state.
+            final Object reinstateWitness = LOCKED_TX.compareAndExchange(this, null, tx);
+            verify(reinstateWitness == null, "Reinstating transaction %s collided with locked transaction %s", tx,
+                reinstateWitness);
+            return false;
+        }
+
+        if (!backendCancelled) {
+            LOG.warn("Backend transaction cannot be cancelled during cancellation of {}, attempting to continue", tx);
         }
 
         // There are multiple frontend transactions in this batch. We have to report them as failed, which dooms this
         // transaction chain, too. Since we just came off of a locked transaction, we do not have a ready transaction
         // at the moment, but there may be some transaction in-flight. So we proceed to shutdown the backend chain
         // and mark the fact that we should be turning its completion into a failure.
-        deadTx = new SimpleImmutableEntry<>(tx, new CancellationException("Transaction " + frontendTx + " canceled")
-                .fillInStackTrace());
+        deadTx = Map.entry(tx, new CancellationException("Transaction " + frontendTx + " canceled").fillInStackTrace());
         delegate.close();
+        return true;
     }
 
     @Override
@@ -482,12 +492,10 @@ public final class PingPongTransactionChain implements DOMTransactionChain {
 
         @Override
         public boolean cancel() {
-            if (isOpen) {
-                cancelTransaction(tx, this);
+            if (isOpen && cancelTransaction(tx, this)) {
                 isOpen = false;
                 return true;
             }
-
             return false;
         }