X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=blobdiff_plain;f=opendaylight%2Fmd-sal%2Fsal-dom-broker%2Fsrc%2Fmain%2Fjava%2Forg%2Fopendaylight%2Fcontroller%2Fmd%2Fsal%2Fdom%2Fbroker%2Fimpl%2FPingPongTransactionChain.java;h=9895ff9ad5e87af9901b7ffe03b4a2e82726b307;hb=66d39ecc3effd52c96c7a772a46612008e34fbc9;hp=abd348a9c73404b79f0866160f336bdee30aab59;hpb=1e80b656857bf829d8ae3cae21b0b726190b96ea;p=controller.git diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/PingPongTransactionChain.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/PingPongTransactionChain.java index abd348a9c7..9895ff9ad5 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/PingPongTransactionChain.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/md/sal/dom/broker/impl/PingPongTransactionChain.java @@ -64,7 +64,6 @@ public final class PingPongTransactionChain implements DOMTransactionChain { */ private static final AtomicReferenceFieldUpdater READY_UPDATER = AtomicReferenceFieldUpdater.newUpdater(PingPongTransactionChain.class, PingPongTransaction.class, "readyTx"); - @SuppressWarnings("unused") // Accessed via READY_UPDATER private volatile PingPongTransaction readyTx; /** @@ -157,12 +156,18 @@ public final class PingPongTransactionChain implements DOMTransactionChain { return oldTx; } - // This forces allocateTransaction() on a slow path + /* + * This forces allocateTransaction() on a slow path, which has to happen after + * this method has completed executing. Also inflightTx may be updated outside + * the lock, hence we need to re-check. + */ @GuardedBy("this") private void processIfReady() { - final PingPongTransaction tx = READY_UPDATER.getAndSet(this, null); - if (tx != null) { - processTransaction(tx); + if (inflightTx == null) { + final PingPongTransaction tx = READY_UPDATER.getAndSet(this, null); + if (tx != null) { + processTransaction(tx); + } } } @@ -222,27 +227,54 @@ public final class PingPongTransactionChain implements DOMTransactionChain { } private void readyTransaction(final @Nonnull PingPongTransaction tx) { + // First mark the transaction as not locked. final boolean lockedMatch = LOCKED_UPDATER.compareAndSet(this, tx, null); Preconditions.checkState(lockedMatch, "Attempted to submit transaction %s while we have %s", tx, lockedTx); - LOG.debug("Transaction {} unlocked", tx); + /* + * The transaction is ready. It will then be picked up by either next allocation, + * or a background transaction completion callback. + */ + final boolean success = READY_UPDATER.compareAndSet(this, null, tx); + Preconditions.checkState(success, "Transaction %s collided on ready state", tx, readyTx); + LOG.debug("Transaction {} readied", tx); + + /* + * We do not see a transaction being in-flight, so we need to take care of dispatching + * the transaction to the backend. We are in the ready case, we cannot short-cut + * the checking of readyTx, as an in-flight transaction may have completed between us + * setting the field above and us checking. + */ if (inflightTx == null) { synchronized (this) { - processTransaction(tx); + processIfReady(); } } } @Override - public void close() { + public synchronized void close() { final PingPongTransaction notLocked = lockedTx; Preconditions.checkState(notLocked == null, "Attempted to close chain with outstanding transaction %s", notLocked); - synchronized (this) { - processIfReady(); - delegate.close(); + // Force allocations on slow path. We will complete the rest + final PingPongTransaction tx = READY_UPDATER.getAndSet(this, null); + + // Make sure no transaction is outstanding. Otherwise sleep a bit and retry + while (inflightTx != null) { + LOG.debug("Busy-waiting for in-flight transaction {} to complete", inflightTx); + Thread.yield(); + continue; } + + // If we have an outstanding transaction, send it down + if (tx != null) { + processTransaction(tx); + } + + // All done, close the delegate. All new allocations should fail. + delegate.close(); } @Override