Fix infinite loop on cancel transaction

This patch fixes a problem where you would run into an infinite loop
after cancelling DOMForwardedWriteTransaction following an exception
thrown by the backed transaction ready or submit methods.

Change-Id: I14bcca6727ab2f173d481b84742c4edbf7bf9dd8
JIRA: CONTROLLER-1812
Signed-off-by: Jaime Caamaño Ruiz <jcaamano@suse.com>
(cherry picked from commit 3534aba923cb1d2dcdd9e3ecf2675e35ce343910)

diff --git a/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/DOMForwardedWriteTransaction.java b/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/DOMForwardedWriteTransaction.java
index 6292f82e3add44f7fe31f24de44eba68ba09b2fc..2c9cb721c1a61f6ec0a6b780261fdfe755a348c7 100644 (file)
--- a/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/DOMForwardedWriteTransaction.java
+++ b/dom/mdsal-dom-broker/src/main/java/org/opendaylight/mdsal/dom/broker/DOMForwardedWriteTransaction.java
@@ -124,6 +124,7 @@ class DOMForwardedWriteTransaction<T extends DOMStoreWriteTransaction> extends
     }
 
     @Override
+    @SuppressWarnings("checkstyle:illegalcatch")
     public CheckedFuture<Void, TransactionCommitFailedException> submit() {
         final AbstractDOMForwardedTransactionFactory<?> impl = IMPL_UPDATER.getAndSet(this, null);
         checkRunning(impl);
@@ -131,12 +132,17 @@ class DOMForwardedWriteTransaction<T extends DOMStoreWriteTransaction> extends
         final Collection<T> txns = getSubtransactions();
         final Collection<DOMStoreThreePhaseCommitCohort> cohorts = new ArrayList<>(txns.size());
 
-        // FIXME: deal with errors thrown by backed (ready and submit can fail in theory)
-        for (final DOMStoreWriteTransaction txn : txns) {
-            cohorts.add(txn.ready());
-        }
+        CheckedFuture<Void, TransactionCommitFailedException> ret;
+        try {
+            for (final DOMStoreWriteTransaction txn : txns) {
+                cohorts.add(txn.ready());
+            }
 
-        final CheckedFuture<Void, TransactionCommitFailedException> ret = impl.submit(this, cohorts);
+            ret = impl.submit(this, cohorts);
+        } catch (RuntimeException e) {
+            ret = Futures.immediateFailedCheckedFuture(
+                    TransactionCommitFailedExceptionMapper.COMMIT_ERROR_MAPPER.apply(e));
+        }
         FUTURE_UPDATER.lazySet(this, ret);
         return ret;
     }

diff --git a/dom/mdsal-dom-broker/src/test/java/org/opendaylight/mdsal/dom/broker/DOMForwardedWriteTransactionTest.java b/dom/mdsal-dom-broker/src/test/java/org/opendaylight/mdsal/dom/broker/DOMForwardedWriteTransactionTest.java
new file mode 100644 (file)
index 0000000..dc37003
--- /dev/null
+++ b/dom/mdsal-dom-broker/src/test/java/org/opendaylight/mdsal/dom/broker/DOMForwardedWriteTransactionTest.java
@@ -0,0 +1,76 @@
+/*
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.mdsal.dom.broker;
+
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
+
+import com.google.common.util.concurrent.CheckedFuture;
+import java.util.Collections;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
+import org.opendaylight.mdsal.common.api.TransactionCommitFailedException;
+import org.opendaylight.mdsal.dom.spi.store.DOMStoreWriteTransaction;
+
+public class DOMForwardedWriteTransactionTest {
+
+    @Mock
+    private AbstractDOMForwardedTransactionFactory abstractDOMForwardedTransactionFactory;
+
+    @Mock
+    private DOMStoreWriteTransaction domStoreWriteTransaction;
+
+    @Before
+    public void setup() {
+        MockitoAnnotations.initMocks(this);
+    }
+
+    @Test
+    public void readyRuntimeExceptionAndCancel() {
+        RuntimeException thrown = new RuntimeException();
+        doThrow(thrown).when(domStoreWriteTransaction).ready();
+        DOMForwardedWriteTransaction<DOMStoreWriteTransaction> domForwardedWriteTransaction =
+                new DOMForwardedWriteTransaction<>(
+                        new Object(),
+                        Collections.singletonMap(LogicalDatastoreType.OPERATIONAL, domStoreWriteTransaction),
+                        abstractDOMForwardedTransactionFactory);
+        CheckedFuture<Void, TransactionCommitFailedException> submitFuture = domForwardedWriteTransaction.submit();
+        try {
+            submitFuture.checkedGet();
+            Assert.fail("TransactionCommitFailedException expected");
+        } catch (TransactionCommitFailedException e) {
+            assertTrue(e.getCause() == thrown);
+            domForwardedWriteTransaction.cancel();
+        }
+    }
+
+    @Test
+    public void submitRuntimeExceptionAndCancel() {
+        RuntimeException thrown = new RuntimeException();
+        doReturn(null).when(domStoreWriteTransaction).ready();
+        doThrow(thrown).when(abstractDOMForwardedTransactionFactory).submit(any(), any());
+        DOMForwardedWriteTransaction<DOMStoreWriteTransaction> domForwardedWriteTransaction =
+                new DOMForwardedWriteTransaction<>(
+                        new Object(),
+                        Collections.singletonMap(LogicalDatastoreType.OPERATIONAL, domStoreWriteTransaction),
+                        abstractDOMForwardedTransactionFactory);
+        CheckedFuture<Void, TransactionCommitFailedException> submitFuture = domForwardedWriteTransaction.submit();
+        try {
+            submitFuture.checkedGet();
+            Assert.fail("TransactionCommitFailedException expected");
+        } catch (TransactionCommitFailedException e) {
+            assertTrue(e.getCause() == thrown);
+            domForwardedWriteTransaction.cancel();
+        }
+    }
+}
\ No newline at end of file