Skip to content

Commit 45a8655

Browse files
committed
Fix: Detect infinite update loops caused by render phase updates (#26625)
This PR contains a regression test and two separate fixes: a targeted fix, and a more general one that's designed as a last-resort guard against these types of bugs (both bugs in app code and bugs in React). I confirmed that each of these fixes separately are sufficient to fix the regression test I added. We can't realistically detect all infinite update loop scenarios because they could be async; even a single microtask can foil our attempts to detect a cycle. But this improves our strategy for detecting the most common kind. See commit messages for more details. DiffTrain build for commit 822386f.
1 parent 8afda46 commit 45a8655

File tree

13 files changed

+899
-227
lines changed

13 files changed

+899
-227
lines changed

compiled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react-test-renderer/cjs/ReactTestRenderer-dev.js

Lines changed: 149 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* @noflow
88
* @nolint
99
* @preventMunge
10-
* @generated SignedSource<<bfa37b9235cbe10193422349b5fb18a5>>
10+
* @generated SignedSource<<658af937953acc877e6c9c23bdafbf60>>
1111
*/
1212

1313
'use strict';
@@ -1584,7 +1584,7 @@ function createLaneMap(initial) {
15841584

15851585
return laneMap;
15861586
}
1587-
function markRootUpdated(root, updateLane) {
1587+
function markRootUpdated$1(root, updateLane) {
15881588
root.pendingLanes |= updateLane; // If there are any suspended transitions, it's possible this new update
15891589
// could unblock them. Clear the suspended lanes so that we can try rendering
15901590
// them again.
@@ -1617,7 +1617,7 @@ function markRootSuspended$1(root, suspendedLanes) {
16171617
lanes &= ~lane;
16181618
}
16191619
}
1620-
function markRootPinged(root, pingedLanes) {
1620+
function markRootPinged$1(root, pingedLanes) {
16211621
root.pingedLanes |= root.suspendedLanes & pingedLanes;
16221622
}
16231623
function markRootFinished(root, remainingLanes) {
@@ -6004,6 +6004,21 @@ function ensureRootIsScheduled(root) {
60046004
ReactCurrentActQueue$2.didScheduleLegacyUpdate = true;
60056005
}
60066006
}
6007+
6008+
function unscheduleAllRoots() {
6009+
// This is only done in a fatal error situation, as a last resort to prevent
6010+
// an infinite render loop.
6011+
var root = firstScheduledRoot;
6012+
6013+
while (root !== null) {
6014+
var next = root.next;
6015+
root.next = null;
6016+
root = next;
6017+
}
6018+
6019+
firstScheduledRoot = lastScheduledRoot = null;
6020+
}
6021+
60076022
function flushSyncWorkOnAllRoots() {
60086023
// This is allowed to be called synchronously, but the caller should check
60096024
// the execution context first.
@@ -6032,11 +6047,49 @@ function flushSyncWorkAcrossRoots_impl(onlyLegacy) {
60326047
var workInProgressRootRenderLanes = getWorkInProgressRootRenderLanes(); // There may or may not be synchronous work scheduled. Let's check.
60336048

60346049
var didPerformSomeWork;
6050+
var nestedUpdatePasses = 0;
60356051
var errors = null;
60366052
isFlushingWork = true;
60376053

60386054
do {
6039-
didPerformSomeWork = false;
6055+
didPerformSomeWork = false; // This outer loop re-runs if performing sync work on a root spawns
6056+
// additional sync work. If it happens too many times, it's very likely
6057+
// caused by some sort of infinite update loop. We already have a loop guard
6058+
// in place that will trigger an error on the n+1th update, but it's
6059+
// possible for that error to get swallowed if the setState is called from
6060+
// an unexpected place, like during the render phase. So as an added
6061+
// precaution, we also use a guard here.
6062+
//
6063+
// Ideally, there should be no known way to trigger this synchronous loop.
6064+
// It's really just here as a safety net.
6065+
//
6066+
// This limit is slightly larger than the one that throws inside setState,
6067+
// because that one is preferable because it includes a componens stack.
6068+
6069+
if (++nestedUpdatePasses > 60) {
6070+
// This is a fatal error, so we'll unschedule all the roots.
6071+
unscheduleAllRoots(); // TODO: Change this error message to something different to distinguish
6072+
// it from the one that is thrown from setState. Those are less fatal
6073+
// because they usually will result in the bad component being unmounted,
6074+
// and an error boundary being triggered, rather than us having to
6075+
// forcibly stop the entire scheduler.
6076+
6077+
var infiniteUpdateError = new Error(
6078+
"Maximum update depth exceeded. This can happen when a component " +
6079+
"repeatedly calls setState inside componentWillUpdate or " +
6080+
"componentDidUpdate. React limits the number of nested updates to " +
6081+
"prevent infinite loops."
6082+
);
6083+
6084+
if (errors === null) {
6085+
errors = [infiniteUpdateError];
6086+
} else {
6087+
errors.push(infiniteUpdateError);
6088+
}
6089+
6090+
break;
6091+
}
6092+
60406093
var root = firstScheduledRoot;
60416094

60426095
while (root !== null) {
@@ -19938,7 +19991,13 @@ var workInProgressRootPingedLanes = NoLanes; // Errors that are thrown during th
1993819991
var workInProgressRootConcurrentErrors = null; // These are errors that we recovered from without surfacing them to the UI.
1993919992
// We will log them once the tree commits.
1994019993

19941-
var workInProgressRootRecoverableErrors = null; // The most recent time we either committed a fallback, or when a fallback was
19994+
var workInProgressRootRecoverableErrors = null; // Tracks when an update occurs during the render phase.
19995+
19996+
var workInProgressRootDidIncludeRecursiveRenderUpdate = false; // Thacks when an update occurs during the commit phase. It's a separate
19997+
// variable from the one for renders because the commit phase may run
19998+
// concurrently to a render phase.
19999+
20000+
var didIncludeCommitPhaseUpdate = false; // The most recent time we either committed a fallback, or when a fallback was
1994220001
// filled in with the resolved UI. This lets us throttle the appearance of new
1994320002
// content as it streams in, to minimize jank.
1994420003
// TODO: Think of a better name for this variable?
@@ -20420,7 +20479,8 @@ function finishConcurrentRender(root, exitStatus, finishedWork, lanes) {
2042020479
commitRoot(
2042120480
root,
2042220481
workInProgressRootRecoverableErrors,
20423-
workInProgressTransitions
20482+
workInProgressTransitions,
20483+
workInProgressRootDidIncludeRecursiveRenderUpdate
2042420484
);
2042520485
} else {
2042620486
if (includesOnlyRetries(lanes) && alwaysThrottleRetries) {
@@ -20450,6 +20510,7 @@ function finishConcurrentRender(root, exitStatus, finishedWork, lanes) {
2045020510
finishedWork,
2045120511
workInProgressRootRecoverableErrors,
2045220512
workInProgressTransitions,
20513+
workInProgressRootDidIncludeRecursiveRenderUpdate,
2045320514
lanes
2045420515
),
2045520516
msUntilTimeout
@@ -20463,6 +20524,7 @@ function finishConcurrentRender(root, exitStatus, finishedWork, lanes) {
2046320524
finishedWork,
2046420525
workInProgressRootRecoverableErrors,
2046520526
workInProgressTransitions,
20527+
workInProgressRootDidIncludeRecursiveRenderUpdate,
2046620528
lanes
2046720529
);
2046820530
}
@@ -20473,6 +20535,7 @@ function commitRootWhenReady(
2047320535
finishedWork,
2047420536
recoverableErrors,
2047520537
transitions,
20538+
didIncludeRenderPhaseUpdate,
2047620539
lanes
2047720540
) {
2047820541
// TODO: Combine retry throttling with Suspensey commits. Right now they run
@@ -20496,14 +20559,20 @@ function commitRootWhenReady(
2049620559
// us that it's ready. This will be canceled if we start work on the
2049720560
// root again.
2049820561
root.cancelPendingCommit = schedulePendingCommit(
20499-
commitRoot.bind(null, root, recoverableErrors, transitions)
20562+
commitRoot.bind(
20563+
null,
20564+
root,
20565+
recoverableErrors,
20566+
transitions,
20567+
didIncludeRenderPhaseUpdate
20568+
)
2050020569
);
2050120570
markRootSuspended(root, lanes);
2050220571
return;
2050320572
}
2050420573
} // Otherwise, commit immediately.
2050520574

20506-
commitRoot(root, recoverableErrors, transitions);
20575+
commitRoot(root, recoverableErrors, transitions, didIncludeRenderPhaseUpdate);
2050720576
}
2050820577

2050920578
function isRenderConsistentWithExternalStores(finishedWork) {
@@ -20566,18 +20635,49 @@ function isRenderConsistentWithExternalStores(finishedWork) {
2056620635
// eslint-disable-next-line no-unreachable
2056720636

2056820637
return true;
20638+
} // The extra indirections around markRootUpdated and markRootSuspended is
20639+
// needed to avoid a circular dependency between this module and
20640+
// ReactFiberLane. There's probably a better way to split up these modules and
20641+
// avoid this problem. Perhaps all the root-marking functions should move into
20642+
// the work loop.
20643+
20644+
function markRootUpdated(root, updatedLanes) {
20645+
markRootUpdated$1(root, updatedLanes); // Check for recursive updates
20646+
20647+
if (executionContext & RenderContext) {
20648+
workInProgressRootDidIncludeRecursiveRenderUpdate = true;
20649+
} else if (executionContext & CommitContext) {
20650+
didIncludeCommitPhaseUpdate = true;
20651+
}
20652+
20653+
throwIfInfiniteUpdateLoopDetected();
20654+
}
20655+
20656+
function markRootPinged(root, pingedLanes) {
20657+
markRootPinged$1(root, pingedLanes); // Check for recursive pings. Pings are conceptually different from updates in
20658+
// other contexts but we call it an "update" in this context because
20659+
// repeatedly pinging a suspended render can cause a recursive render loop.
20660+
// The relevant property is that it can result in a new render attempt
20661+
// being scheduled.
20662+
20663+
if (executionContext & RenderContext) {
20664+
workInProgressRootDidIncludeRecursiveRenderUpdate = true;
20665+
} else if (executionContext & CommitContext) {
20666+
didIncludeCommitPhaseUpdate = true;
20667+
}
20668+
20669+
throwIfInfiniteUpdateLoopDetected();
2056920670
}
2057020671

2057120672
function markRootSuspended(root, suspendedLanes) {
2057220673
// When suspending, we should always exclude lanes that were pinged or (more
2057320674
// rarely, since we try to avoid it) updated during the render phase.
20574-
// TODO: Lol maybe there's a better way to factor this besides this
20575-
// obnoxiously named function :)
2057620675
suspendedLanes = removeLanes(suspendedLanes, workInProgressRootPingedLanes);
2057720676
suspendedLanes = removeLanes(
2057820677
suspendedLanes,
2057920678
workInProgressRootInterleavedUpdatedLanes
2058020679
);
20680+
2058120681
markRootSuspended$1(root, suspendedLanes);
2058220682
} // This is the entry point for synchronous tasks that don't go
2058320683
// through Scheduler
@@ -20648,7 +20748,8 @@ function performSyncWorkOnRoot(root) {
2064820748
commitRoot(
2064920749
root,
2065020750
workInProgressRootRecoverableErrors,
20651-
workInProgressTransitions
20751+
workInProgressTransitions,
20752+
workInProgressRootDidIncludeRecursiveRenderUpdate
2065220753
); // Before exiting, make sure there's a callback scheduled for the next
2065320754
// pending level.
2065420755

@@ -20789,6 +20890,7 @@ function prepareFreshStack(root, lanes) {
2078920890
workInProgressRootPingedLanes = NoLanes;
2079020891
workInProgressRootConcurrentErrors = null;
2079120892
workInProgressRootRecoverableErrors = null;
20893+
workInProgressRootDidIncludeRecursiveRenderUpdate = false;
2079220894
finishQueueingConcurrentUpdates();
2079320895

2079420896
{
@@ -21685,7 +21787,12 @@ function unwindUnitOfWork(unitOfWork) {
2168521787
workInProgress = null;
2168621788
}
2168721789

21688-
function commitRoot(root, recoverableErrors, transitions) {
21790+
function commitRoot(
21791+
root,
21792+
recoverableErrors,
21793+
transitions,
21794+
didIncludeRenderPhaseUpdate
21795+
) {
2168921796
// TODO: This no longer makes any sense. We already wrap the mutation and
2169021797
// layout phases. Should be able to remove.
2169121798
var previousUpdateLanePriority = getCurrentUpdatePriority();
@@ -21698,6 +21805,7 @@ function commitRoot(root, recoverableErrors, transitions) {
2169821805
root,
2169921806
recoverableErrors,
2170021807
transitions,
21808+
didIncludeRenderPhaseUpdate,
2170121809
previousUpdateLanePriority
2170221810
);
2170321811
} finally {
@@ -21712,6 +21820,7 @@ function commitRootImpl(
2171221820
root,
2171321821
recoverableErrors,
2171421822
transitions,
21823+
didIncludeRenderPhaseUpdate,
2171521824
renderPriorityLevel
2171621825
) {
2171721826
do {
@@ -21767,7 +21876,9 @@ function commitRootImpl(
2176721876

2176821877
var concurrentlyUpdatedLanes = getConcurrentlyUpdatedLanes();
2176921878
remainingLanes = mergeLanes(remainingLanes, concurrentlyUpdatedLanes);
21770-
markRootFinished(root, remainingLanes);
21879+
markRootFinished(root, remainingLanes); // Reset this before firing side effects so we can detect recursive updates.
21880+
21881+
didIncludeCommitPhaseUpdate = false;
2177121882

2177221883
if (root === workInProgressRoot) {
2177321884
// We can reset these now that they are finished.
@@ -21948,7 +22059,18 @@ function commitRootImpl(
2194822059

2194922060
remainingLanes = root.pendingLanes;
2195022061

21951-
if (includesSyncLane(remainingLanes)) {
22062+
if (
22063+
// Check if there was a recursive update spawned by this render, in either
22064+
// the render phase or the commit phase. We track these explicitly because
22065+
// we can't infer from the remaining lanes alone.
22066+
didIncludeCommitPhaseUpdate ||
22067+
didIncludeRenderPhaseUpdate || // As an additional precaution, we also check if there's any remaining sync
22068+
// work. Theoretically this should be unreachable but if there's a mistake
22069+
// in React it helps to be overly defensive given how hard it is to debug
22070+
// those scenarios otherwise. This won't catch recursive async updates,
22071+
// though, which is why we check the flags above first.
22072+
includesSyncLane(remainingLanes)
22073+
) {
2195222074
{
2195322075
markNestedUpdateScheduled();
2195422076
} // Count the number of times the root synchronously re-renders without
@@ -22386,6 +22508,18 @@ function throwIfInfiniteUpdateLoopDetected() {
2238622508
nestedPassiveUpdateCount = 0;
2238722509
rootWithNestedUpdates = null;
2238822510
rootWithPassiveNestedUpdates = null;
22511+
22512+
if (executionContext & RenderContext && workInProgressRoot !== null) {
22513+
// We're in the render phase. Disable the concurrent error recovery
22514+
// mechanism to ensure that the error we're about to throw gets handled.
22515+
// We need it to trigger the nearest error boundary so that the infinite
22516+
// update loop is broken.
22517+
workInProgressRoot.errorRecoveryDisabledLanes = mergeLanes(
22518+
workInProgressRoot.errorRecoveryDisabledLanes,
22519+
workInProgressRootRenderLanes
22520+
);
22521+
}
22522+
2238922523
throw new Error(
2239022524
"Maximum update depth exceeded. This can happen when a component " +
2239122525
"repeatedly calls setState inside componentWillUpdate or " +
@@ -23857,7 +23991,7 @@ function createFiberRoot(
2385723991
return root;
2385823992
}
2385923993

23860-
var ReactVersion = "18.3.0-canary-80d9a4011-20230627";
23994+
var ReactVersion = "18.3.0-canary-822386f25-20230627";
2386123995

2386223996
// Might add PROFILE later.
2386323997

0 commit comments

Comments
 (0)