Skip to content

[Flight] Fix visitAsyncNode infinite recursion with circular async nodes #35246

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jere-co wants to merge 2 commits into from
Closed

[Flight] Fix visitAsyncNode infinite recursion with circular async nodes #35246

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
76f529d
[Flight] Fix visitAsyncNode infinite recursion with circular async nodes
jere-co Nov 28, 2025
2224817
Merge branch 'main' into fix/visitAsyncNode-infinite-recursion
eps1lon Jan 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 26 additions & 12 deletions packages/react-server/src/ReactFlightServer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2320,28 +2320,42 @@ function renderElement(
return renderClientElement(request, task, type, key, props, validated);
}

// Sentinel to mark nodes currently being evaluated.
// Distinguishes "in progress" from a real cached result (including undefined/null).
const IN_PROGRESS: symbol = Symbol.for('react.asyncTraversal.inProgress');

function visitAsyncNode(
request: Request,
task: Task,
node: AsyncSequence,
visited: Map<
AsyncSequence | ReactDebugInfo,
void | null | PromiseNode | IONode,
void | null | PromiseNode | IONode | symbol,
>,
cutOff: number,
): void | null | PromiseNode | IONode {
if (visited.has(node)) {
// It's possible to visit them same node twice when it's part of both an "awaited" path
// and a "previous" path. This also gracefully handles cycles which would be a bug.
return visited.get(node);
const memo = visited.get(node);
if (memo === IN_PROGRESS) {
// Cycle detected: we're currently evaluating this node further up the call stack.
// Return null to indicate no I/O was found on this cyclic path. We don't return
// undefined here because that signals "abort" semantics which would skip emitting
// I/O info for other non-cyclic branches of this node.
return null;
Copy link
Collaborator

@unstubbable unstubbable Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the PR description you write:

Returns null on cycle detection to indicate "no I/O found on this cyclic path" (not undefined, which would signal abort semantics and skip emitting I/O info for other non-cyclic branches)

However, this doesn't actually fix the reported issue. I've synced your fix to the repro app and it still runs into a stack overflow. The fix just appears to work because it's not applied correctly here:

https://github.com/jere-co/next-debug/blob/25c992b4e021765343067a30d8b70fa20a337448/patch-visitAsyncNode.cjs#L128

That patch returns undefined instead of null. If we did that, we'd drop I/O info for valid cases, which you can see with the failing unit tests that this change would trigger.

Also, I think the IN_PROGRESS sentinel as implemented here doesn't actually change the cycle detection behavior. The original code already returns null for a cycle (via visited.get(node) returning the null that was set before recursing). The sentinel would only matter if we wanted to return something different on cycle detection.

It seems like the DB library is producing very long linear chains of async nodes via previous pointers, and the stack overflow comes from the recursive traversal of these chains, not from undetected cycles. A proper fix might require converting the previous chain traversal from recursive to iterative, though this gets complicated because the traversal also needs to handle awaited branches and return values need to propagate correctly.

Copy link
Collaborator

@unstubbable unstubbable Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this maybe: #35612

}
// It's possible to visit the same node twice when it's part of both an "awaited" path
// and a "previous" path. Return the cached result.
return memo;
}
// Set it as visited early in case we see ourselves before returning.
visited.set(node, null);
// Mark as in progress before descending so cycles short-circuit.
visited.set(node, IN_PROGRESS);

const result = visitAsyncNodeImpl(request, task, node, visited, cutOff);
if (result !== null) {
// If we ended up with a value, let's use that value for future visits.
visited.set(node, result);
}

// Cache the exact result for future visits (including undefined/null).
// This ensures revisits observe the real computed value, not the IN_PROGRESS marker.
visited.set(node, result);

return result;
}

Expand All @@ -2351,7 +2365,7 @@ function visitAsyncNodeImpl(
node: AsyncSequence,
visited: Map<
AsyncSequence | ReactDebugInfo,
void | null | PromiseNode | IONode,
void | null | PromiseNode | IONode | symbol,
>,
cutOff: number,
): void | null | PromiseNode | IONode {
Expand Down Expand Up @@ -2593,7 +2607,7 @@ function emitAsyncSequence(
): void {
const visited: Map<
AsyncSequence | ReactDebugInfo,
void | null | PromiseNode | IONode,
void | null | PromiseNode | IONode | symbol,
> = new Map();
if (__DEV__ && alreadyForwardedDebugInfo) {
visited.set(alreadyForwardedDebugInfo, null);
Expand Down
Loading