mirror of
https://github.com/facebook/react.git
synced 2026-02-26 04:55:00 +00:00
[Flight] Fix stack overflow in visitAsyncNode with deep async chains (#35612)
Database libraries like Gel/EdgeDB can create very long linear chains of async sequences through temporal async sequencing in connection pools. The recursive traversal of `node.previous` chains in `visitAsyncNode` causes stack overflow on these deep chains. The fix converts the `previous` chain traversal from recursive to iterative. We collect the chain into an array, then process from deepest to shallowest. The `awaited` traversal remains recursive since its depth is bounded by promise dependency depth, not by the number of event loop turns. Each `awaited` branch still benefits from the iterative `previous` handling within its own traversal. I've verified that this fixes the [repro](https://github.com/jere-co/next-debug) provided in #35246. closes #35246
This commit is contained in:
74
packages/react-server/src/ReactFlightServer.js
vendored
74
packages/react-server/src/ReactFlightServer.js
vendored
@@ -2345,19 +2345,53 @@ function visitAsyncNode(
|
||||
>,
|
||||
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);
|
||||
// Collect the previous chain iteratively instead of recursively to avoid
|
||||
// stack overflow on deep chains. We process from deepest to shallowest so
|
||||
// each node has its previousIONode available.
|
||||
const chain: Array<AsyncSequence> = [];
|
||||
let current: AsyncSequence | null = node;
|
||||
|
||||
while (current !== null) {
|
||||
if (visited.has(current)) {
|
||||
break;
|
||||
}
|
||||
chain.push(current);
|
||||
current = current.previous;
|
||||
}
|
||||
// Set it as visited early in case we see ourselves before returning.
|
||||
visited.set(node, null);
|
||||
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);
|
||||
|
||||
let previousIONode: void | null | PromiseNode | IONode =
|
||||
current !== null ? visited.get(current) : null;
|
||||
|
||||
// Process from deepest to shallowest (reverse order).
|
||||
for (let i = chain.length - 1; i >= 0; i--) {
|
||||
const n = chain[i];
|
||||
// Set it as visited early in case we see the node again before returning.
|
||||
visited.set(n, null);
|
||||
|
||||
const result = visitAsyncNodeImpl(
|
||||
request,
|
||||
task,
|
||||
n,
|
||||
visited,
|
||||
cutOff,
|
||||
previousIONode,
|
||||
);
|
||||
|
||||
if (result !== null) {
|
||||
// If we ended up with a value, let's use that value for future visits.
|
||||
visited.set(n, result);
|
||||
}
|
||||
|
||||
if (result === undefined) {
|
||||
// Undefined is used as a signal that we found a suitable aborted node
|
||||
// and we don't have to find further aborted nodes.
|
||||
return undefined;
|
||||
}
|
||||
|
||||
previousIONode = result;
|
||||
}
|
||||
return result;
|
||||
|
||||
return previousIONode;
|
||||
}
|
||||
|
||||
function visitAsyncNodeImpl(
|
||||
@@ -2369,6 +2403,7 @@ function visitAsyncNodeImpl(
|
||||
void | null | PromiseNode | IONode,
|
||||
>,
|
||||
cutOff: number,
|
||||
previousIONode: void | null | PromiseNode | IONode,
|
||||
): void | null | PromiseNode | IONode {
|
||||
if (node.end >= 0 && node.end <= request.timeOrigin) {
|
||||
// This was already resolved when we started this render. It must have been either something
|
||||
@@ -2377,23 +2412,6 @@ function visitAsyncNodeImpl(
|
||||
return null;
|
||||
}
|
||||
|
||||
let previousIONode: void | null | PromiseNode | IONode = null;
|
||||
// First visit anything that blocked this sequence to start in the first place.
|
||||
if (node.previous !== null) {
|
||||
previousIONode = visitAsyncNode(
|
||||
request,
|
||||
task,
|
||||
node.previous,
|
||||
visited,
|
||||
cutOff,
|
||||
);
|
||||
if (previousIONode === undefined) {
|
||||
// Undefined is used as a signal that we found a suitable aborted node and we don't have to find
|
||||
// further aborted nodes.
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
// `found` represents the return value of the following switch statement.
|
||||
// We can't use multiple `return` statements in the switch statement
|
||||
// since that prevents Closure compiler from inlining `visitAsyncImpl`
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user