OutOfMemoryError caused by infinite loop inside StateNode.visitNodeTreeBottomUp()

[ignacio-alvarez]

Description of the bug

For several weeks we had frequent application crashes caused by an OutOfMemoryError. It appears it is being caused by an infinite loop inside StateNode.visitNodeTreeBottomUp():

flow/flow-server/src/main/java/com/vaadin/flow/internal/StateNode.java

Line 702 in 3be853c

void visitNodeTreeBottomUp(Consumer<StateNode> visitor) {

The infinite loop happens because the implementation depends on a child node pointing to the correct parent, line 714:

previousParent = current.getParent();

But what happens if a child node points to a different parent? What happens if getParent() is null? If that happens to the first child node, it enters into an infinite loop because the condition if (current == previousParent) never becomes true, and the loop alternates between removing the child node and adding it again infinitely.

The OutOfMemoryError occurs because the infinite loop is triggered during an onDetach() event, and visitor.accept() expands its internal ArrayList until it runs out of memory.

This error is difficult to reproduce because it is actually an implementation flaw that is triggered by other random hidden bugs, that sometimes cause a child node to not point to its parent. We had crashes as frequently as every few hours, and sometimes we went days without triggering it.

We fixed the problem by replacing visitNodeTreeBottomUp() with a recursive function so that it doesn't depend on getParent() anymore. We've read why there are no recursive functions in StateNode, but we have no need for nesting thousands of nodes, and if we do, we can just increase the thread stack size:
#252

We wanted to report this issue in case there are other users having the same problem.

Thank you.

Expected behavior

Application shouldn't enter into an infinite loop and crash with an OutOfMemoryError.

Minimal reproducible example

This bug is difficult to reproduce, because it depends on other bugs that are triggered randomly. We had crashes as frequently as every few hours, and sometimes we went days without triggering it.

Versions

• Vaadin / Flow version: 23.0.0
• Java version: International Business Machines Corporation 11.0.12
• OS version: amd64 Linux 5.15.23-76051523-generic
• Browser version (if applicable): Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:98.0) Gecko/20100101 Firefox/98.0
• Application Server (if applicable):
• IDE (if applicable):

[knoobie]

This feels kinda related to another refactoring I've observed in the past #12665

[ignacio-alvarez]

@caalador Just an observation, there should be a childrenAdded.add(this) after line 708, to prevent the root node from being iterated twice.

[vaadin-bot]

This ticket/PR has been released with Vaadin 14.8.7.

[vaadin-bot]

This ticket/PR has been released with Vaadin 22.0.12.

[vaadin-bot]

This ticket/PR has been released with Vaadin 23.0.4.