Fix infinite loop on cancel transaction

[controller.git] / opendaylight / md-sal / sal-distributed-datastore / src / main / java / org / opendaylight / controller / cluster / databroker / AbstractDOMBrokerWriteTransaction.java

diff --git a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/AbstractDOMBrokerWriteTransaction.java b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/AbstractDOMBrokerWriteTransaction.java
index 500ccdf5d587dd3b5d753c93a51d42fb8bbaba7d..cfc6c283856f8810e67430de02aaab9a45fd9aa3 100644 (file)
--- a/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/AbstractDOMBrokerWriteTransaction.java
+++ b/opendaylight/md-sal/sal-distributed-datastore/src/main/java/org/opendaylight/controller/cluster/databroker/AbstractDOMBrokerWriteTransaction.java
@@ -22,6 +22,7 @@ import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException;
 import org.opendaylight.controller.md.sal.common.impl.service.AbstractDataTransaction;
 import org.opendaylight.controller.md.sal.dom.api.DOMDataWriteTransaction;
+import org.opendaylight.controller.md.sal.dom.broker.impl.TransactionCommitFailedExceptionMapper;
 import org.opendaylight.controller.sal.core.spi.data.DOMStoreThreePhaseCommitCohort;
 import org.opendaylight.controller.sal.core.spi.data.DOMStoreTransactionFactory;
 import org.opendaylight.controller.sal.core.spi.data.DOMStoreWriteTransaction;
@@ -35,10 +36,14 @@ import org.slf4j.LoggerFactory;
 public abstract class AbstractDOMBrokerWriteTransaction<T extends DOMStoreWriteTransaction>
         extends AbstractDOMBrokerTransaction<T> implements DOMDataWriteTransaction {
 
-    private static final AtomicReferenceFieldUpdater<AbstractDOMBrokerWriteTransaction, AbstractDOMTransactionFactory> IMPL_UPDATER =
-            AtomicReferenceFieldUpdater.newUpdater(AbstractDOMBrokerWriteTransaction.class, AbstractDOMTransactionFactory.class, "commitImpl");
+    @SuppressWarnings("rawtypes")
+    private static final AtomicReferenceFieldUpdater<AbstractDOMBrokerWriteTransaction, AbstractDOMTransactionFactory>
+            IMPL_UPDATER = AtomicReferenceFieldUpdater.newUpdater(AbstractDOMBrokerWriteTransaction.class,
+                    AbstractDOMTransactionFactory.class, "commitImpl");
+    @SuppressWarnings("rawtypes")
     private static final AtomicReferenceFieldUpdater<AbstractDOMBrokerWriteTransaction, Future> FUTURE_UPDATER =
-            AtomicReferenceFieldUpdater.newUpdater(AbstractDOMBrokerWriteTransaction.class, Future.class, "commitFuture");
+            AtomicReferenceFieldUpdater.newUpdater(AbstractDOMBrokerWriteTransaction.class, Future.class,
+                    "commitFuture");
     private static final Logger LOG = LoggerFactory.getLogger(AbstractDOMBrokerWriteTransaction.class);
     private static final Future<?> CANCELLED_FUTURE = Futures.immediateCancelledFuture();
 
@@ -53,7 +58,7 @@ public abstract class AbstractDOMBrokerWriteTransaction<T extends DOMStoreWriteT
      * Future task of transaction commit. It starts off as null, but is
      * set appropriately on {@link #submit()} and {@link #cancel()} via
      * {@link AtomicReferenceFieldUpdater#lazySet(Object, Object)}.
-     *
+     * <p/>
      * Lazy set is safe for use because it is only referenced to in the
      * {@link #cancel()} slow path, where we will busy-wait for it. The
      * fast path gets the benefit of a store-store barrier instead of the
@@ -62,13 +67,15 @@ public abstract class AbstractDOMBrokerWriteTransaction<T extends DOMStoreWriteT
     private volatile Future<?> commitFuture;
 
     protected AbstractDOMBrokerWriteTransaction(final Object identifier,
-                                            final Map<LogicalDatastoreType, ? extends DOMStoreTransactionFactory> storeTxFactories, final AbstractDOMTransactionFactory<?> commitImpl) {
+            final Map<LogicalDatastoreType, ? extends DOMStoreTransactionFactory> storeTxFactories,
+            final AbstractDOMTransactionFactory<?> commitImpl) {
         super(identifier, storeTxFactories);
         this.commitImpl = Preconditions.checkNotNull(commitImpl, "commitImpl must not be null.");
     }
 
     @Override
-    public void put(final LogicalDatastoreType store, final YangInstanceIdentifier path, final NormalizedNode<?, ?> data) {
+    public void put(final LogicalDatastoreType store, final YangInstanceIdentifier path,
+            final NormalizedNode<?, ?> data) {
         checkRunning(commitImpl);
         checkInstanceIdentifierReferencesData(path,data);
         getSubtransaction(store).write(path, data);
@@ -76,9 +83,10 @@ public abstract class AbstractDOMBrokerWriteTransaction<T extends DOMStoreWriteT
 
     private static void checkInstanceIdentifierReferencesData(final YangInstanceIdentifier path,
             final NormalizedNode<?, ?> data) {
+        Preconditions.checkArgument(data != null, "Attempted to store null data at %s", path);
         final PathArgument lastArg = path.getLastPathArgument();
         Preconditions.checkArgument(
-                (lastArg == data.getIdentifier()) || (lastArg != null && lastArg.equals(data.getIdentifier())),
+                lastArg == data.getIdentifier() || lastArg != null && lastArg.equals(data.getIdentifier()),
                 "Instance identifier references %s but data identifier is %s", lastArg, data);
     }
 
@@ -89,7 +97,8 @@ public abstract class AbstractDOMBrokerWriteTransaction<T extends DOMStoreWriteT
     }
 
     @Override
-    public void merge(final LogicalDatastoreType store, final YangInstanceIdentifier path, final NormalizedNode<?, ?> data) {
+    public void merge(final LogicalDatastoreType store, final YangInstanceIdentifier path,
+            final NormalizedNode<?, ?> data) {
         checkRunning(commitImpl);
         checkInstanceIdentifierReferencesData(path, data);
         getSubtransaction(store).merge(path, data);
@@ -110,7 +119,8 @@ public abstract class AbstractDOMBrokerWriteTransaction<T extends DOMStoreWriteT
         Future<?> future;
         do {
             future = commitFuture;
-        } while (future == null);
+        }
+        while (future == null);
 
         return future.cancel(false);
     }
@@ -122,6 +132,7 @@ public abstract class AbstractDOMBrokerWriteTransaction<T extends DOMStoreWriteT
     }
 
     @Override
+    @SuppressWarnings("checkstyle:illegalcatch")
     public CheckedFuture<Void, TransactionCommitFailedException> submit() {
         final AbstractDOMTransactionFactory<?> impl = IMPL_UPDATER.getAndSet(this, null);
         checkRunning(impl);
@@ -129,12 +140,17 @@ public abstract class AbstractDOMBrokerWriteTransaction<T extends DOMStoreWriteT
         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 T txn : txns) {
-            cohorts.add(txn.ready());
-        }
+        CheckedFuture<Void, TransactionCommitFailedException> ret;
+        try {
+            for (final T 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;
     }