From 3534aba923cb1d2dcdd9e3ecf2675e35ce343910 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Jaime=20Caama=C3=B1o=20Ruiz?= Date: Fri, 9 Feb 2018 10:47:26 +0100 Subject: [PATCH 1/1] Fix infinite loop on cancel transaction MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 --- .../broker/DOMForwardedWriteTransaction.java | 16 ++-- .../DOMForwardedWriteTransactionTest.java | 76 +++++++++++++++++++ 2 files changed, 87 insertions(+), 5 deletions(-) create mode 100644 dom/mdsal-dom-broker/src/test/java/org/opendaylight/mdsal/dom/broker/DOMForwardedWriteTransactionTest.java 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 6292f82e3a..2c9cb721c1 100644 --- 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 extends } @Override + @SuppressWarnings("checkstyle:illegalcatch") public CheckedFuture submit() { final AbstractDOMForwardedTransactionFactory impl = IMPL_UPDATER.getAndSet(this, null); checkRunning(impl); @@ -131,12 +132,17 @@ class DOMForwardedWriteTransaction extends final Collection txns = getSubtransactions(); final Collection 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 ret; + try { + for (final DOMStoreWriteTransaction txn : txns) { + cohorts.add(txn.ready()); + } - final CheckedFuture 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 index 0000000000..dc37003a4d --- /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 domForwardedWriteTransaction = + new DOMForwardedWriteTransaction<>( + new Object(), + Collections.singletonMap(LogicalDatastoreType.OPERATIONAL, domStoreWriteTransaction), + abstractDOMForwardedTransactionFactory); + CheckedFuture 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 domForwardedWriteTransaction = + new DOMForwardedWriteTransaction<>( + new Object(), + Collections.singletonMap(LogicalDatastoreType.OPERATIONAL, domStoreWriteTransaction), + abstractDOMForwardedTransactionFactory); + CheckedFuture 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 -- 2.36.6