Skip to content

Spawn new task if we hit stack overflow #30419

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

Merged
merged 1 commit into from
Aug 27, 2024
+137 −16
Merged

Spawn new task if we hit stack overflow #30419

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
61 changes: 61 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8218,4 +8218,65 @@ describe('ReactDOMFizzServer', () => {
'\n in Bar (at **)' + '\n in Foo (at **)',
);
});

it('can recover from very deep trees to avoid stack overflow', async () => {
function Recursive({n}) {
if (n > 0) {
return <Recursive n={n - 1} />;
}
return <span>hi</span>;
}

// Recursively render a component tree deep enough to trigger stack overflow.
// Don't make this too short to not hit the limit but also not too deep to slow
// down the test.
await act(() => {
const {pipe} = renderToPipeableStream(
<div>
<Recursive n={1000} />
</div>,
);
pipe(writable);
});

expect(getVisibleChildren(container)).toEqual(
<div>
<span>hi</span>
</div>,
);
});

it('handles stack overflows inside components themselves', async () => {
function StackOverflow() {
// This component is recursive inside itself and is therefore an error.
// Assuming no tail-call optimizations.
function recursive(n, a0, a1, a2, a3) {
if (n > 0) {
return recursive(n - 1, a0, a1, a2, a3) + a0 + a1 + a2 + a3;
}
return a0;
}
return recursive(10000, 'should', 'not', 'resolve', 'this');
}

let caughtError;

await expect(async () => {
await act(() => {
const {pipe} = renderToPipeableStream(
<div>
<StackOverflow />
</div>,
{
onError(error, errorInfo) {
caughtError = error;
},
},
);
pipe(writable);
});
}).rejects.toThrow('Maximum call stack size exceeded');

expect(caughtError.message).toBe('Maximum call stack size exceeded');
});
});
92 changes: 76 additions & 16 deletions packages/react-server/src/ReactFizzServer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3231,9 +3231,8 @@ function spawnNewSuspendedReplayTask(
request: Request,
task: ReplayTask,
thenableState: ThenableState | null,
x: Wakeable,
): void {
const newTask = createReplayTask(
): ReplayTask {
return createReplayTask(
request,
thenableState,
task.replay,
Expand All @@ -3251,17 +3250,13 @@ function spawnNewSuspendedReplayTask(
!disableLegacyContext ? task.legacyContext : emptyContextObject,
__DEV__ && enableOwnerStacks ? task.debugTask : null,
);

const ping = newTask.ping;
x.then(ping, ping);
}

function spawnNewSuspendedRenderTask(
request: Request,
task: RenderTask,
thenableState: ThenableState | null,
x: Wakeable,
): void {
): RenderTask {
// Something suspended, we'll need to create a new segment and resolve it later.
const segment = task.blockedSegment;
const insertionIndex = segment.chunks.length;
Expand All @@ -3278,7 +3273,7 @@ function spawnNewSuspendedRenderTask(
segment.children.push(newSegment);
// Reset lastPushedText for current Segment since the new Segment "consumed" it
segment.lastPushedText = false;
const newTask = createRenderTask(
return createRenderTask(
request,
thenableState,
task.node,
Expand All @@ -3296,9 +3291,6 @@ function spawnNewSuspendedRenderTask(
!disableLegacyContext ? task.legacyContext : emptyContextObject,
__DEV__ && enableOwnerStacks ? task.debugTask : null,
);

const ping = newTask.ping;
x.then(ping, ping);
}

// This is a non-destructive form of rendering a node. If it suspends it spawns
Expand Down Expand Up @@ -3347,14 +3339,48 @@ function renderNode(
if (typeof x.then === 'function') {
const wakeable: Wakeable = (x: any);
const thenableState = getThenableStateAfterSuspending();
spawnNewSuspendedReplayTask(
const newTask = spawnNewSuspendedReplayTask(
request,
// $FlowFixMe: Refined.
task,
thenableState,
);
const ping = newTask.ping;
wakeable.then(ping, ping);

// Restore the context. We assume that this will be restored by the inner
// functions in case nothing throws so we don't use "finally" here.
task.formatContext = previousFormatContext;
if (!disableLegacyContext) {
task.legacyContext = previousLegacyContext;
}
task.context = previousContext;
task.keyPath = previousKeyPath;
task.treeContext = previousTreeContext;
task.componentStack = previousComponentStack;
if (__DEV__ && enableOwnerStacks) {
task.debugTask = previousDebugTask;
}
// Restore all active ReactContexts to what they were before.
switchContext(previousContext);
return;
}
if (x.message === 'Maximum call stack size exceeded') {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

V8 and Hermes has the same message.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we log some warning here? Is there any way product developers can action on this / any other downsides to having trees deep enough to hit this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know if it's actionable. If you have a large site like facebook you're likely to have giant trees. However, you might also not have the ability to increase the stack limit of the service you're using.

// This was a stack overflow. We do a lot of recursion in React by default for
// performance but it can lead to stack overflows in extremely deep trees.
// We do have the ability to create a trampoile if this happens which makes
// this kind of zero-cost.
const thenableState = getThenableStateAfterSuspending();
const newTask = spawnNewSuspendedReplayTask(
request,
// $FlowFixMe: Refined.
task,
thenableState,
wakeable,
);

// Immediately schedule the task for retrying.
request.pingedTasks.push(newTask);

// Restore the context. We assume that this will be restored by the inner
// functions in case nothing throws so we don't use "finally" here.
task.formatContext = previousFormatContext;
Expand Down Expand Up @@ -3404,13 +3430,14 @@ function renderNode(
if (typeof x.then === 'function') {
const wakeable: Wakeable = (x: any);
const thenableState = getThenableStateAfterSuspending();
spawnNewSuspendedRenderTask(
const newTask = spawnNewSuspendedRenderTask(
request,
// $FlowFixMe: Refined.
task,
thenableState,
wakeable,
);
const ping = newTask.ping;
wakeable.then(ping, ping);

// Restore the context. We assume that this will be restored by the inner
// functions in case nothing throws so we don't use "finally" here.
Expand Down Expand Up @@ -3451,6 +3478,39 @@ function renderNode(
);
trackPostpone(request, trackedPostpones, task, postponedSegment);

// Restore the context. We assume that this will be restored by the inner
// functions in case nothing throws so we don't use "finally" here.
task.formatContext = previousFormatContext;
if (!disableLegacyContext) {
task.legacyContext = previousLegacyContext;
}
task.context = previousContext;
task.keyPath = previousKeyPath;
task.treeContext = previousTreeContext;
task.componentStack = previousComponentStack;
if (__DEV__ && enableOwnerStacks) {
task.debugTask = previousDebugTask;
}
// Restore all active ReactContexts to what they were before.
switchContext(previousContext);
return;
}
if (x.message === 'Maximum call stack size exceeded') {
// This was a stack overflow. We do a lot of recursion in React by default for
// performance but it can lead to stack overflows in extremely deep trees.
// We do have the ability to create a trampoile if this happens which makes
// this kind of zero-cost.
const thenableState = getThenableStateAfterSuspending();
const newTask = spawnNewSuspendedRenderTask(
request,
// $FlowFixMe: Refined.
task,
thenableState,
);

// Immediately schedule the task for retrying.
request.pingedTasks.push(newTask);

// Restore the context. We assume that this will be restored by the inner
// functions in case nothing throws so we don't use "finally" here.
task.formatContext = previousFormatContext;
Expand Down