Skip to content

Commit

Permalink
Stop resetting outside of render
Browse files Browse the repository at this point in the history
  • Loading branch information
eps1lon committed Mar 9, 2025
1 parent 97ffdbb commit 2ee6f47
Show file tree
Hide file tree
Showing 9 changed files with 108 additions and 45 deletions.
3 changes: 1 addition & 2 deletions packages/internal-test-utils/internalAct.js
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,7 @@ export async function serverAct<T>(scope: () => Thenable<T>): Thenable<T> {

// $FlowFixMe[cannot-resolve-name]: Flow doesn't know about global Jest object
const j = jest;
// We have always one pending timer running that resets the Owner Stack limit.
if (j.getTimerCount() > 1) {
if (j.getTimerCount() > 0) {
// There's a pending timer. Flush it now. We only do this in order to
// force Suspense fallbacks to display; the fact that it's a timer
// is an implementation detail. If there are other timers scheduled,
Expand Down
10 changes: 10 additions & 0 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ import {
enableViewTransition,
enableSwipeTransition,
} from 'shared/ReactFeatureFlags';
import {
startResettingOwnerStackLimit,
stopResettingOwnerStackLimit,
} from 'shared/ReactOwnerStackReset';
import ReactSharedInternals from 'shared/ReactSharedInternals';
import is from 'shared/objectIs';

Expand Down Expand Up @@ -1320,6 +1324,10 @@ function finishConcurrentRender(
}
}

if (__DEV__) {
stopResettingOwnerStackLimit();
}

if (shouldForceFlushFallbacksInDEV()) {
// We're inside an `act` scope. Commit immediately.
commitRoot(
Expand Down Expand Up @@ -1984,6 +1992,8 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber {
finishQueueingConcurrentUpdates();

if (__DEV__) {
startResettingOwnerStackLimit();

ReactStrictModeWarnings.discardPendingWarnings();
}

Expand Down
39 changes: 17 additions & 22 deletions packages/react-reconciler/src/__tests__/ReactOwnerStacks-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,10 @@ describe('ReactOwnerStacks', () => {
let i = 0;
i <
siblingsBeforeStackOne -
// One built-in JSX callsite for the unknown Owner Stack
1 -
// <App /> callsite
1 -
// JSX callsites before this are irrelevant since we always reset the limit
// when we start a render. Both <App /> and <UnknownOwner /> were created
// before we actually start rendering.
0 -
// Stop so that OwnerStackOne will be right before cutoff
1;
i++
Expand Down Expand Up @@ -149,8 +149,7 @@ describe('ReactOwnerStacks', () => {
stackOne: normalizeCodeLocInfo(stackOne),
stackTwo: normalizeCodeLocInfo(stackTwo),
}).toEqual({
// We continue resetting periodically.
pendingTimers: 1,
pendingTimers: 0,
stackOne: '\n in App (at **)',
stackTwo: __VARIANT__
? // captured right after cutoff
Expand All @@ -176,8 +175,7 @@ describe('ReactOwnerStacks', () => {
stackOne: normalizeCodeLocInfo(stackOne),
stackTwo: normalizeCodeLocInfo(stackTwo),
}).toEqual({
// We continue resetting periodically.
pendingTimers: 1,
pendingTimers: 0,
// rendered everything before cutoff
stackOne: '\n in App (at **)',
stackTwo: '\n in App (at **)',
Expand All @@ -192,10 +190,10 @@ describe('ReactOwnerStacks', () => {
let i = 0;
i <
siblingsBeforeStackOne -
// One built-in JSX callsite for the unknown Owner Stack
1 -
// <App /> callsite
1 -
// JSX callsites before this are irrelevant since we always reset the limit
// when we start a render. Both <App /> and <UnknownOwner /> were created
// before we actually start rendering.
0 -
// Stop so that OwnerStackOne will be right before cutoff
1;
i++
Expand Down Expand Up @@ -273,8 +271,7 @@ describe('ReactOwnerStacks', () => {
stackOne: normalizeCodeLocInfo(stackOne),
stackTwo: normalizeCodeLocInfo(stackTwo),
}).toEqual({
// 1 for periodically resetting the Owner Stack limit
pendingTimers: 1,
pendingTimers: 0,
stackOne: '\n in App (at **)',
stackTwo:
// captured after we reset the limit
Expand All @@ -295,10 +292,10 @@ describe('ReactOwnerStacks', () => {
let i = 0;
i <
siblingsBeforeStackOne -
// One built-in JSX callsite for the unknown Owner Stack
1 -
// <App /> callsite
1 -
// JSX callsites before this are irrelevant since we always reset the limit
// when we start a render. Both <App /> and <UnknownOwner /> were created
// before we actually start rendering.
0 -
// Stop so that OwnerStackOne will be right before cutoff
1;
i++
Expand Down Expand Up @@ -357,8 +354,7 @@ describe('ReactOwnerStacks', () => {
stackTwo: normalizeCodeLocInfo(stackTwo),
}).toEqual({
// 1 for the timeout
// 1 for periodically resetting the Owner Stack limit
pendingTimers: 2,
pendingTimers: 1,
stackOne: '\n in App (at **)',
stackTwo: undefined,
});
Expand All @@ -372,8 +368,7 @@ describe('ReactOwnerStacks', () => {
stackOne: normalizeCodeLocInfo(stackOne),
stackTwo: normalizeCodeLocInfo(stackTwo),
}).toEqual({
// 1 for periodically resetting the Owner Stack limit
pendingTimers: 1,
pendingTimers: 0,
stackOne: '\n in App (at **)',
stackTwo:
// captured after we reset the limit
Expand Down
9 changes: 9 additions & 0 deletions packages/react-server/src/ReactFizzServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ import {
callRenderInDEV,
} from './ReactFizzCallUserSpace';

import {
startResettingOwnerStackLimit,
stopResettingOwnerStackLimit,
} from 'shared/ReactOwnerStackReset';
import {
getIteratorFn,
ASYNC_ITERATOR,
Expand Down Expand Up @@ -4571,6 +4575,8 @@ export function performWork(request: Request): void {
if (__DEV__) {
prevGetCurrentStackImpl = ReactSharedInternals.getCurrentStack;
ReactSharedInternals.getCurrentStack = getCurrentStackInDEV;

startResettingOwnerStackLimit();
}
const prevResumableState = currentResumableState;
setCurrentResumableState(request.resumableState);
Expand All @@ -4591,10 +4597,13 @@ export function performWork(request: Request): void {
fatalError(request, error, errorInfo, null);
} finally {
setCurrentResumableState(prevResumableState);

ReactSharedInternals.H = prevDispatcher;
ReactSharedInternals.A = prevAsyncDispatcher;

if (__DEV__) {
stopResettingOwnerStackLimit();

ReactSharedInternals.getCurrentStack = prevGetCurrentStackImpl;
}
if (prevDispatcher === HooksDispatcher) {
Expand Down
12 changes: 12 additions & 0 deletions packages/react-server/src/ReactFlightServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ import {DefaultAsyncDispatcher} from './flight/ReactFlightAsyncDispatcher';
import {resolveOwner, setCurrentOwner} from './flight/ReactFlightCurrentOwner';

import {getOwnerStackByComponentInfoInDev} from 'shared/ReactComponentInfoStack';
import {
startResettingOwnerStackLimit,
stopResettingOwnerStackLimit,
} from 'shared/ReactOwnerStackReset';

import {
callComponentInDEV,
Expand Down Expand Up @@ -4058,6 +4062,10 @@ function performWork(request: Request): void {
currentRequest = request;
prepareToUseHooksForRequest(request);

if (__DEV__) {
startResettingOwnerStackLimit();
}

const hadAbortableTasks = request.abortableTasks.size > 0;
try {
const pingedTasks = request.pingedTasks;
Expand All @@ -4080,6 +4088,10 @@ function performWork(request: Request): void {
logRecoverableError(request, error, null);
fatalError(request, error);
} finally {
if (__DEV__) {
stopResettingOwnerStackLimit();
}

ReactSharedInternals.H = prevDispatcher;
resetHooksForRequest();
currentRequest = prevRequest;
Expand Down
11 changes: 5 additions & 6 deletions packages/react-server/src/__tests__/ReactFlightServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,10 @@ describe('ReactFlight', () => {
let i = 0;
i <
siblingsBeforeStackOne -
// One built-in JSX callsite for the unknown Owner Stack
1 -
// <App /> callsite
1 -
// JSX callsites before this are irrelevant since we always reset the limit
// when we start a render. Both <App /> and <UnknownOwner /> were created
// before we actually start rendering.
0 -
// Stop so that OwnerStackOne will be right before cutoff
1;
i++
Expand Down Expand Up @@ -143,8 +143,7 @@ describe('ReactFlight', () => {
stackOne: normalizeCodeLocInfo(stackOne),
stackTwo: normalizeCodeLocInfo(stackTwo),
}).toEqual({
// 1 for periodically resetting the Owner Stack limit
pendingTimers: 1,
pendingTimers: 0,
stackOne: '\n in App (at **)',
stackTwo:
// captured after we reset the limit
Expand Down
8 changes: 1 addition & 7 deletions packages/react/src/ReactSharedInternalsClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export type SharedStateClient = {
// ReactDebugCurrentFrame
getCurrentStack: null | (() => string),

// ReactOwnerStackReset
recentlyCreatedOwnerStacks: 0,
};

Expand All @@ -60,14 +61,7 @@ if (__DEV__) {
ReactSharedInternals.thrownErrors = [];
// Stack implementation injected by the current renderer.
ReactSharedInternals.getCurrentStack = (null: null | (() => string));
// ReactOwnerStack
ReactSharedInternals.recentlyCreatedOwnerStacks = 0;
if (typeof setInterval === 'function') {
// TODO: Stop outside of rendering.
setInterval(() => {
ReactSharedInternals.recentlyCreatedOwnerStacks = 0;
}, 1000);
}
}

export default ReactSharedInternals;
9 changes: 1 addition & 8 deletions packages/react/src/ReactSharedInternalsServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export type SharedStateServer = {
// ReactDebugCurrentFrame
getCurrentStack: null | (() => string),

//
// ReactOwnerStackReset
recentlyCreatedOwnerStacks: 0,
};

Expand All @@ -62,14 +62,7 @@ if (enableTaint) {
if (__DEV__) {
// Stack implementation injected by the current renderer.
ReactSharedInternals.getCurrentStack = (null: null | (() => string));
// ReactOwnerStack
ReactSharedInternals.recentlyCreatedOwnerStacks = 0;
if (typeof setInterval === 'function') {
// TODO: Stop outside of rendering.
setInterval(() => {
ReactSharedInternals.recentlyCreatedOwnerStacks = 0;
}, 1000);
}
}

export default ReactSharedInternals;
52 changes: 52 additions & 0 deletions packages/shared/ReactOwnerStackReset.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/**
* Copyright (c) Meta Platforms, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

import ReactSharedInternals from 'shared/ReactSharedInternals';

let resetOwnerStackIntervalId: mixed = null;

export function startResettingOwnerStackLimit() {
if (__DEV__) {
ReactSharedInternals.recentlyCreatedOwnerStacks = 0;

if (typeof setInterval === 'function') {
// Renderers might start in render but stop in commit.
// So we need to be resilient against start being called multiple times.
if (resetOwnerStackIntervalId !== null) {
clearInterval((resetOwnerStackIntervalId: any));
}
resetOwnerStackIntervalId = setInterval(() => {
ReactSharedInternals.recentlyCreatedOwnerStacks = 0;
}, 1000);
}
} else {
// These errors should never make it into a build so we don't need to encode them in codes.json
// eslint-disable-next-line react-internal/prod-error-codes
throw new Error(
'startResettingOwnerStackLimit should never be called in production mode. This is a bug in React.',
);
}
}

export function stopResettingOwnerStackLimit() {
if (__DEV__) {
if (typeof setInterval === 'function') {
if (resetOwnerStackIntervalId !== null) {
clearInterval((resetOwnerStackIntervalId: any));
resetOwnerStackIntervalId = null;
}
}
} else {
// These errors should never make it into a build so we don't need to encode them in codes.json
// eslint-disable-next-line react-internal/prod-error-codes
throw new Error(
'stopResettingOwnerStackLimit should never be called in production mode. This is a bug in React.',
);
}
}

0 comments on commit 2ee6f47

Please sign in to comment.