Do not synchronize around ReactorStmtCtx.schemaPath
[yangtools.git] / yang / yang-parser-reactor / src / main / java / org / opendaylight / yangtools / yang / parser / stmt / reactor / StatementContextBase.java
index 02fb8d222e5f01c48c967bfd1d8b25fc280f421f..a9aa4099c3c1ac48d09962bb526a19b5a00153b2 100644 (file)
@@ -140,7 +140,10 @@ public abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E
 
     private Multimap<ModelProcessingPhase, OnPhaseFinished> phaseListeners = ImmutableMultimap.of();
     private Multimap<ModelProcessingPhase, ContextMutation> phaseMutation = ImmutableMultimap.of();
+
+    // Note: this field is accessed either directly, or under substatementsInitialized == true
     private List<StatementContextBase<?, ?, ?>> effective = ImmutableList.of();
+
     private List<StmtContext<?, ?, ?>> effectOfStatement = ImmutableList.of();
 
     private @Nullable ModelProcessingPhase completedPhase;
@@ -163,7 +166,7 @@ public abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E
 
     // SchemaPath cache for use with SubstatementContext and InferredStatementContext. This hurts RootStatementContext
     // a bit in terms of size -- but those are only a few and SchemaPath is on its way out anyway.
-    private volatile SchemaPath schemaPath;
+    private SchemaPath schemaPath;
 
     // Copy constructor used by subclasses to implement reparent()
     StatementContextBase(final StatementContextBase<A, D, E> original) {
@@ -371,7 +374,8 @@ public abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E
     }
 
     @Override
-    public Collection<? extends Mutable<?, ?, ?>> mutableEffectiveSubstatements() {
+    public final Collection<? extends Mutable<?, ?, ?>> mutableEffectiveSubstatements() {
+        ensureEffectiveSubstatements();
         if (effective instanceof ImmutableCollection) {
             return effective;
         }
@@ -380,12 +384,15 @@ public abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E
     }
 
     private void shrinkEffective() {
+        // Initialization guarded by all callers
         if (effective.isEmpty()) {
             effective = ImmutableList.of();
         }
     }
 
-    public void removeStatementFromEffectiveSubstatements(final StatementDefinition statementDef) {
+    // Note: has side-effect of ensureEffectiveSubstatements()
+    public final void removeStatementFromEffectiveSubstatements(final StatementDefinition statementDef) {
+        ensureEffectiveSubstatements();
         if (effective.isEmpty()) {
             return;
         }
@@ -413,10 +420,13 @@ public abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E
      * @param statementDef statement definition of the statement context to remove
      * @param statementArg statement argument of the statement context to remove
      */
-    public void removeStatementFromEffectiveSubstatements(final StatementDefinition statementDef,
+    public final void removeStatementFromEffectiveSubstatements(final StatementDefinition statementDef,
             final String statementArg) {
         if (statementArg == null) {
+            // Note: has side-effect of ensureEffectiveSubstatements()
             removeStatementFromEffectiveSubstatements(statementDef);
+        } else {
+            ensureEffectiveSubstatements();
         }
 
         if (effective.isEmpty()) {
@@ -456,8 +466,9 @@ public abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E
      * @throws NullPointerException
      *             if statement parameter is null
      */
-    public void addEffectiveSubstatement(final Mutable<?, ?, ?> substatement) {
+    public final void addEffectiveSubstatement(final Mutable<?, ?, ?> substatement) {
         verifyStatement(substatement);
+        ensureEffectiveSubstatements();
         beforeAddEffectiveStatement(1);
 
         final StatementContextBase<?, ?, ?> stmt = (StatementContextBase<?, ?, ?>) substatement;
@@ -477,14 +488,25 @@ public abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E
      * @throws NullPointerException
      *             if statement parameter is null
      */
-    public void addEffectiveSubstatements(final Collection<? extends Mutable<?, ?, ?>> statements) {
+    public final void addEffectiveSubstatements(final Collection<? extends Mutable<?, ?, ?>> statements) {
         if (!statements.isEmpty()) {
             statements.forEach(StatementContextBase::verifyStatement);
+            ensureEffectiveSubstatements();
             beforeAddEffectiveStatement(statements.size());
             doAddEffectiveSubstatements(statements);
         }
     }
 
+    // exposed for InferredStatementContext, which we expect to initialize effective substatements
+    void ensureEffectiveSubstatements() {
+        // No-op for everything except InferredStatementContext
+    }
+
+    // Exposed for InferredStatementContextr only, others do not need initialization
+    Iterable<StatementContextBase<?, ?, ?>> effectiveChildrenToComplete() {
+        return effective;
+    }
+
     // exposed for InferredStatementContext only
     final void addInitialEffectiveSubstatements(final Collection<? extends Mutable<?, ?, ?>> statements) {
         verify(!substatementsInitialized, "Attempted to re-initialized statement {} with {}", this, statements);
@@ -507,6 +529,7 @@ public abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E
             }
         }
 
+        // Initialization guarded by all callers
         effective.addAll(casted);
     }
 
@@ -535,6 +558,7 @@ public abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E
                 || inProgressPhase == ModelProcessingPhase.EFFECTIVE_MODEL,
                 "Effective statement cannot be added in declared phase at: %s", getStatementSourceReference());
 
+        // Initialization guarded by all callers
         if (effective.isEmpty()) {
             effective = new ArrayList<>(toAdd);
         }
@@ -594,7 +618,7 @@ public abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E
         for (final StatementContextBase<?, ?, ?> child : mutableDeclaredSubstatements()) {
             finished &= child.tryToCompletePhase(phase);
         }
-        for (final StatementContextBase<?, ?, ?> child : effective) {
+        for (final StatementContextBase<?, ?, ?> child : effectiveChildrenToComplete()) {
             finished &= child.tryToCompletePhase(phase);
         }
         return finished;
@@ -619,13 +643,17 @@ public abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E
 
         if (openMutations.isEmpty()) {
             phaseMutation.removeAll(phase);
-            if (phaseMutation.isEmpty()) {
-                phaseMutation = ImmutableMultimap.of();
-            }
+            cleanupPhaseMutation();
         }
         return finished;
     }
 
+    private void cleanupPhaseMutation() {
+        if (phaseMutation.isEmpty()) {
+            phaseMutation = ImmutableMultimap.of();
+        }
+    }
+
     /**
      * Occurs on end of {@link ModelProcessingPhase} of source parsing.
      *
@@ -802,10 +830,9 @@ public abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E
     /**
      * Adds a {@link ContextMutation} to a {@link ModelProcessingPhase}.
      *
-     * @throws IllegalStateException
-     *             when the mutation was registered after phase was completed
+     * @throws IllegalStateException when the mutation was registered after phase was completed
      */
-    void addMutation(final ModelProcessingPhase phase, final ContextMutation mutation) {
+    final void addMutation(final ModelProcessingPhase phase, final ContextMutation mutation) {
         ModelProcessingPhase finishedPhase = completedPhase;
         while (finishedPhase != null) {
             checkState(!phase.equals(finishedPhase), "Mutation registered after phase was completed at: %s",
@@ -819,6 +846,13 @@ public abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E
         phaseMutation.put(phase, mutation);
     }
 
+    final void removeMutation(final ModelProcessingPhase phase, final ContextMutation mutation) {
+        if (!phaseMutation.isEmpty()) {
+            phaseMutation.remove(phase, mutation);
+            cleanupPhaseMutation();
+        }
+    }
+
     @Override
     public <K, KT extends K, N extends StatementNamespace<K, ?, ?>> void addContext(final Class<@NonNull N> namespace,
             final KT key,final StmtContext<?, ?, ?> stmt) {
@@ -941,6 +975,10 @@ public abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E
      */
     abstract boolean hasEmptySubstatements();
 
+    // Dual use method: AbstractResumedStatement does not use 'initialized' and InferredStatementContext ensures
+    //                  initialization.
+    // FIXME: 7.0.0: I think this warrants a separate subclasses, as InferredStatementContext wants to manage these
+    //               itself. Before we do that, though, we need to analyze size impacts
     final boolean hasEmptyEffectiveSubstatements() {
         return effective.isEmpty();
     }
@@ -1036,17 +1074,10 @@ public abstract class StatementContextBase<A, D extends DeclaredStatement<A>, E
     // Exists only to support {SubstatementContext,InferredStatementContext}.getSchemaPath()
     @Deprecated
     final @NonNull Optional<SchemaPath> substatementGetSchemaPath() {
-        SchemaPath local = schemaPath;
-        if (local == null) {
-            synchronized (this) {
-                local = schemaPath;
-                if (local == null) {
-                    schemaPath = local = createSchemaPath(coerceParentContext());
-                }
-            }
+        if (schemaPath == null) {
+            schemaPath = createSchemaPath(coerceParentContext());
         }
-
-        return Optional.ofNullable(local);
+        return Optional.ofNullable(schemaPath);
     }
 
     @Deprecated