From 61ada97d4d672d5a47a0d4ed17b977d61428752d Mon Sep 17 00:00:00 2001
From: caalador <mikael.grankvist@vaadin.com>
Date: Mon, 28 Mar 2022 17:29:39 +0300
Subject: [PATCH] fix: Break loop in visitor (#13370)

Do not constantly add children
of a node that has already had
all children added once.

Fixes #13366
---
 .../com/vaadin/flow/internal/StateNode.java   |  4 +++
 .../vaadin/flow/internal/StateNodeTest.java   | 30 +++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/flow-server/src/main/java/com/vaadin/flow/internal/StateNode.java b/flow-server/src/main/java/com/vaadin/flow/internal/StateNode.java
index 2dac3ec9ee4..e8de6a91f14 100644
--- a/flow-server/src/main/java/com/vaadin/flow/internal/StateNode.java
+++ b/flow-server/src/main/java/com/vaadin/flow/internal/StateNode.java
@@ -698,6 +698,7 @@ void visitNodeTreeBottomUp(Consumer<StateNode> visitor) {
         // not done inside loop to please Sonarcube
         forEachChild(stack::addFirst);
         StateNode previousParent = this;
+        Set<StateNode> childrenAdded = new HashSet<>();
 
         while (!stack.isEmpty()) {
             StateNode current = stack.getFirst();
@@ -705,8 +706,11 @@ void visitNodeTreeBottomUp(Consumer<StateNode> visitor) {
             if (current == previousParent) {
                 visitor.accept(stack.removeFirst());
                 previousParent = current.getParent();
+            } else if (childrenAdded.contains(current)) {
+                previousParent = current;
             } else {
                 current.forEachChild(stack::addFirst);
+                childrenAdded.add(current);
                 previousParent = current;
             }
         }
diff --git a/flow-server/src/test/java/com/vaadin/flow/internal/StateNodeTest.java b/flow-server/src/test/java/com/vaadin/flow/internal/StateNodeTest.java
index cfcb1294bd2..d3999a6b73c 100644
--- a/flow-server/src/test/java/com/vaadin/flow/internal/StateNodeTest.java
+++ b/flow-server/src/test/java/com/vaadin/flow/internal/StateNodeTest.java
@@ -16,6 +16,7 @@
 
 package com.vaadin.flow.internal;
 
+import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashSet;
@@ -294,6 +295,35 @@ public void nodeTreeOnAttach_bottomUpTraversing_correctOrder() {
                 data.removeLast()));
     }
 
+    @Test
+    public void nodeTreeOnAttach_bottomUpTraversing_brokenParentInChildDoesNotEndInLoop()
+            throws NoSuchFieldException, IllegalAccessException {
+        // Set data is used to track the node during debug see
+        // TestStateNode.toString
+        TestStateNode root = new TestStateNode();
+        root.setData(0);
+        List<Integer> count = new ArrayList<>();
+
+        final Field parent = StateNode.class.getDeclaredField("parent");
+        parent.setAccessible(true);
+
+        TestStateNode childOfRoot = new TestStateNode();
+        childOfRoot.setData(1);
+
+        TestStateNode child = new TestStateNode();
+        child.setData(2);
+        setParent(child, childOfRoot);
+
+        parent.set(child, null);
+
+        setParent(childOfRoot, root);
+
+        root.visitNodeTreeBottomUp(node -> count.add(1));
+
+        Assert.assertEquals("Each node should be visited once", 3,
+                count.size());
+    }
+
     @Test
     public void attachListener_onSetParent_listenerTriggered() {
         StateNode root = new TestStateTree().getRootNode();