From 3d0ba7cc365c4f8dc2c8e52945e75d20bc3ddd51 Mon Sep 17 00:00:00 2001
From: Robert Varga <robert.varga@pantheon.tech>
Date: Sun, 12 Dec 2021 21:11:51 +0100
Subject: [PATCH] Delay modifiers during their processing

When we are invoking actions, we are also iterating over them, hence
we have to stop modifiers being added -- otherwise we'll get a CME.
Add the minimal machinery to make this work.

JIRA: YANGTOOLS-1381
Change-Id: I44c04d7bdafe9fddb0bddd01e5cbb4912d356723
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
---
 .../stmt/reactor/SourceSpecificContext.java   | 34 +++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/SourceSpecificContext.java b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/SourceSpecificContext.java
index 484f291a09..0d1b582140 100644
--- a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/SourceSpecificContext.java
+++ b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/SourceSpecificContext.java
@@ -20,6 +20,7 @@ import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Objects;
 import java.util.Optional;
 import org.eclipse.jdt.annotation.NonNull;
@@ -112,6 +113,9 @@ final class SourceSpecificContext implements NamespaceStorageNode, NamespaceBeha
     private ModelProcessingPhase finishedPhase = ModelProcessingPhase.INIT;
     private ModelProcessingPhase inProgressPhase;
 
+    // If not null, do not add anything to modifiers, but record it here.
+    private List<Entry<ModelProcessingPhase, ModifierImpl>> delayedModifiers;
+
     SourceSpecificContext(final BuildGlobalContext globalContext, final StatementStreamSource source) {
         this.globalContext = requireNonNull(globalContext);
         this.source = requireNonNull(source);
@@ -319,9 +323,13 @@ final class SourceSpecificContext implements NamespaceStorageNode, NamespaceBeha
         return hasProgressed ? PhaseCompletionProgress.PROGRESS : PhaseCompletionProgress.NO_PROGRESS;
     }
 
-    private static boolean tryToProgress(final Collection<ModifierImpl> currentPhaseModifiers) {
+    private boolean tryToProgress(final Collection<ModifierImpl> currentPhaseModifiers) {
         boolean hasProgressed = false;
 
+        // We are about to iterate over the modifiers and invoke callbacks. Those callbacks can end up circling back
+        // and modifying the same collection. This asserts that modifiers should not be modified.
+        delayedModifiers = List.of();
+
         // Try making forward progress ...
         final Iterator<ModifierImpl> modifier = currentPhaseModifiers.iterator();
         while (modifier.hasNext()) {
@@ -331,12 +339,34 @@ final class SourceSpecificContext implements NamespaceStorageNode, NamespaceBeha
             }
         }
 
+        // We have finished iterating, if we have any delayed modifiers, put them back. This may seem as if we want
+        // to retry the loop, but we do not have to, as we will be circling back anyway.
+        //
+        // The thing is, we are inherently single-threaded and therefore if we observe non-empty delayedModifiers, the
+        // only way that could happen is through a callback, which in turn means we have made progress.
+        if (!delayedModifiers.isEmpty()) {
+            verify(hasProgressed, "Delayed modifiers encountered without making progress in %s", this);
+            for (Entry<ModelProcessingPhase, ModifierImpl> entry : delayedModifiers) {
+                modifiers.put(entry.getKey(), entry.getValue());
+            }
+        }
+        delayedModifiers = null;
+
         return hasProgressed;
     }
 
     @NonNull ModelActionBuilder newInferenceAction(final @NonNull ModelProcessingPhase phase) {
         final ModifierImpl action = new ModifierImpl();
-        modifiers.put(phase, action);
+
+        if (delayedModifiers != null) {
+            if (delayedModifiers.isEmpty()) {
+                delayedModifiers = new ArrayList<>(2);
+            }
+            delayedModifiers.add(Map.entry(phase,action));
+        } else {
+            modifiers.put(phase, action);
+        }
+
         return action;
     }
 
-- 
2.36.6