Improve InMemoryDataTreeModification state management

We have an int here, which allows us to express multiple states. Improve
state tracking defensiveness to differentiate between "open" state and
two states ready() goes through.

This means we can detect when ready() ends up not completing
successfully as observed by other validation code.

JIRA: YANGTOOLS-1539
Change-Id: I10a233adf53029d14ba37751cc3cc9f38fc2a85d
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/data/yang-data-tree-ri/src/main/java/org/opendaylight/yangtools/yang/data/tree/impl/AbstractDataTreeTip.java b/data/yang-data-tree-ri/src/main/java/org/opendaylight/yangtools/yang/data/tree/impl/AbstractDataTreeTip.java
index 4628e94d378f871f200540f647b9f43876747ff9..494b5ba483dafd799f640e31c91bed1a9901558f 100644 (file)
--- a/data/yang-data-tree-ri/src/main/java/org/opendaylight/yangtools/yang/data/tree/impl/AbstractDataTreeTip.java
+++ b/data/yang-data-tree-ri/src/main/java/org/opendaylight/yangtools/yang/data/tree/impl/AbstractDataTreeTip.java
@@ -7,9 +7,6 @@
  */
 package org.opendaylight.yangtools.yang.data.tree.impl;
 
-import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkState;
-
 import java.util.Optional;
 import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
@@ -31,32 +28,34 @@ abstract class AbstractDataTreeTip implements DataTreeTip {
 
     @Override
     public final void validate(final DataTreeModification modification) throws DataValidationFailedException {
-        final InMemoryDataTreeModification m = checkedCast(modification);
-        checkArgument(m.isSealed(), "Attempted to verify unsealed modification %s", m);
-
+        final var m = accessMod(modification, "validate");
         m.getStrategy().checkApplicable(new ModificationPath(getRootPath()), m.getRootModification(),
             Optional.of(getTipRoot()), m.getVersion());
     }
 
     @Override
     public final DataTreeCandidateTip prepare(final DataTreeModification modification) {
-        final InMemoryDataTreeModification m = checkedCast(modification);
-        checkArgument(m.isSealed(), "Attempted to prepare unsealed modification %s", m);
+        final var m = accessMod(modification, "prepare");
+        final var root = m.getRootModification();
 
-        final ModifiedNode root = m.getRootModification();
-
-        final TreeNode currentRoot = getTipRoot();
+        final var currentRoot = getTipRoot();
         if (root.getOperation() == LogicalOperation.NONE) {
             return new NoopDataTreeCandidate(YangInstanceIdentifier.of(), root, currentRoot);
         }
 
-        final var newRoot = m.getStrategy().apply(m.getRootModification(), Optional.of(currentRoot), m.getVersion());
-        checkState(newRoot.isPresent(), "Apply strategy failed to produce root node for modification %s", modification);
-        return new InMemoryDataTreeCandidate(YangInstanceIdentifier.of(), root, currentRoot, newRoot.orElseThrow());
+        final var newRoot = m.getStrategy().apply(m.getRootModification(), Optional.of(currentRoot), m.getVersion())
+            .orElseThrow(() -> new IllegalStateException("Apply strategy failed to produce root node for modification "
+                + modification));
+        return new InMemoryDataTreeCandidate(YangInstanceIdentifier.of(), root, currentRoot, newRoot);
     }
 
-    private static InMemoryDataTreeModification checkedCast(final DataTreeModification mod) {
-        checkArgument(mod instanceof InMemoryDataTreeModification, "Invalid modification class %s", mod.getClass());
-        return (InMemoryDataTreeModification)mod;
+    private static @NonNull InMemoryDataTreeModification accessMod(final DataTreeModification mod, final String op) {
+        if (mod instanceof InMemoryDataTreeModification inMemoryMod) {
+            if (inMemoryMod.isSealed()) {
+                return inMemoryMod;
+            }
+            throw new IllegalArgumentException("Attempted to " + op + " unsealed modification " + inMemoryMod);
+        }
+        throw new IllegalArgumentException("Invalid modification " + mod.getClass());
     }
 }

diff --git a/data/yang-data-tree-ri/src/main/java/org/opendaylight/yangtools/yang/data/tree/impl/InMemoryDataTreeModification.java b/data/yang-data-tree-ri/src/main/java/org/opendaylight/yangtools/yang/data/tree/impl/InMemoryDataTreeModification.java
index 6ce2d14c94e9cb1c379304e66e4ad19da44fd9c8..df3da508a0a894e24ba5e6d8aef808ed0b55ce07 100644 (file)
--- a/data/yang-data-tree-ri/src/main/java/org/opendaylight/yangtools/yang/data/tree/impl/InMemoryDataTreeModification.java
+++ b/data/yang-data-tree-ri/src/main/java/org/opendaylight/yangtools/yang/data/tree/impl/InMemoryDataTreeModification.java
@@ -33,24 +33,28 @@ final class InMemoryDataTreeModification extends AbstractCursorAware implements
         EffectiveModelContextProvider {
     private static final Logger LOG = LoggerFactory.getLogger(InMemoryDataTreeModification.class);
 
-    private final RootApplyStrategy strategyTree;
-    private final InMemoryDataTreeSnapshot snapshot;
-    private final ModifiedNode rootNode;
-    private final Version version;
+    private static final byte STATE_OPEN    = 0;
+    private static final byte STATE_SEALING = 1;
+    private static final byte STATE_SEALED  = 2;
 
-    private static final VarHandle SEALED;
+    private static final VarHandle STATE;
 
     static {
         try {
-            SEALED = MethodHandles.lookup().findVarHandle(InMemoryDataTreeModification.class, "sealed", int.class);
+            STATE = MethodHandles.lookup().findVarHandle(InMemoryDataTreeModification.class, "state", byte.class);
         } catch (NoSuchFieldException | IllegalAccessException e) {
             throw new ExceptionInInitializerError(e);
         }
     }
 
-    // All access needs to go through this handle
+    private final RootApplyStrategy strategyTree;
+    private final InMemoryDataTreeSnapshot snapshot;
+    private final ModifiedNode rootNode;
+    private final Version version;
+
+    // All access needs to go through STATE
     @SuppressWarnings("unused")
-    private volatile int sealed;
+    private volatile byte state;
 
     InMemoryDataTreeModification(final InMemoryDataTreeSnapshot snapshot,
             final RootApplyStrategy resolver) {
@@ -223,12 +227,13 @@ final class InMemoryDataTreeModification extends AbstractCursorAware implements
 
     boolean isSealed() {
         // a quick check, synchronizes *only* on the sealed field
-        return (int) SEALED.getAcquire(this) != 0;
+        return (byte) STATE.getAcquire(this) == STATE_SEALED;
     }
 
     private void checkOpen() {
-        if (isSealed()) {
-            throw new IllegalStateException("Data Tree is sealed. No further modifications allowed.");
+        final var local = (byte) STATE.getAcquire(this);
+        if (local != STATE_OPEN) {
+            throw new IllegalStateException("Data Tree is sealed. No further modifications allowed in state " + local);
         }
     }
 
@@ -303,7 +308,7 @@ final class InMemoryDataTreeModification extends AbstractCursorAware implements
     public void ready() {
         // We want a full CAS with setVolatile() memory semantics, as we want to force happen-before for everything,
         // including whatever user code works.
-        if (!SEALED.compareAndSet(this, 0, 1)) {
+        if (!STATE.compareAndSet(this, STATE_OPEN, STATE_SEALING)) {
             throw new IllegalStateException("Attempted to seal an already-sealed Data Tree.");
         }
 
@@ -315,6 +320,6 @@ final class InMemoryDataTreeModification extends AbstractCursorAware implements
         // Make sure all affects are visible before returning, as this object may be handed off to another thread, which
         // needs to see any HashMap.modCount mutations completed. This is needed because isSealed() is now performing
         // only the equivalent of an acquireFence()
-        VarHandle.releaseFence();
+        STATE.setRelease(this, STATE_SEALED);
     }
 }