Skip to content

Commit

Permalink
Add infinite update loop detection (#28279)
Browse files Browse the repository at this point in the history
This is a partial redo of #26625.
Since that was unlanded due to some detected breakages. This now
includes a feature flag to be careful in rolling this out.

DiffTrain build for [d8c1fa6](d8c1fa6)
  • Loading branch information
kassens committed Feb 9, 2024
1 parent b239264 commit e0e832e
Show file tree
Hide file tree
Showing 19 changed files with 1,479 additions and 459 deletions.
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
ba5e6a8329c7194a2c573c037a37f24ce45ee58f
d8c1fa6b0b8da0512cb5acab9cd4f242451392f3
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-prod.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -578,4 +578,4 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactCurrentDispatcher.current.useTransition();
};
exports.version = "18.3.0-www-classic-ab0edb2e";
exports.version = "18.3.0-www-classic-d27287f8";
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-profiling.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactCurrentDispatcher.current.useTransition();
};
exports.version = "18.3.0-www-classic-5d6c9add";
exports.version = "18.3.0-www-classic-08828b5d";
"undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ &&
"function" ===
typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop &&
Expand Down
121 changes: 106 additions & 15 deletions compiled/facebook-www/ReactART-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ if (__DEV__) {
return self;
}

var ReactVersion = "18.3.0-www-classic-e77af7a1";
var ReactVersion = "18.3.0-www-classic-bb651f27";

var LegacyRoot = 0;
var ConcurrentRoot = 1;
Expand Down Expand Up @@ -186,7 +186,9 @@ if (__DEV__) {
retryLaneExpirationMs = dynamicFeatureFlags.retryLaneExpirationMs,
syncLaneExpirationMs = dynamicFeatureFlags.syncLaneExpirationMs,
transitionLaneExpirationMs =
dynamicFeatureFlags.transitionLaneExpirationMs; // On WWW, false is used for a new modern build.
dynamicFeatureFlags.transitionLaneExpirationMs,
enableInfiniteRenderLoopDetection =
dynamicFeatureFlags.enableInfiniteRenderLoopDetection; // On WWW, false is used for a new modern build.
var enableProfilerTimer = true;
var enableProfilerCommitHooks = true;
var enableProfilerNestedUpdatePhase = true;
Expand Down Expand Up @@ -2214,7 +2216,7 @@ if (__DEV__) {

return laneMap;
}
function markRootUpdated(root, updateLane) {
function markRootUpdated$1(root, updateLane) {
root.pendingLanes |= updateLane; // If there are any suspended transitions, it's possible this new update
// could unblock them. Clear the suspended lanes so that we can try rendering
// them again.
Expand Down Expand Up @@ -2251,7 +2253,7 @@ if (__DEV__) {
markSpawnedDeferredLane(root, spawnedLane, suspendedLanes);
}
}
function markRootPinged(root, pingedLanes) {
function markRootPinged$1(root, pingedLanes) {
root.pingedLanes |= root.suspendedLanes & pingedLanes;
}
function markRootFinished(root, remainingLanes, spawnedLane) {
Expand Down Expand Up @@ -25103,7 +25105,13 @@ if (__DEV__) {
var workInProgressRootConcurrentErrors = null; // These are errors that we recovered from without surfacing them to the UI.
// We will log them once the tree commits.

var workInProgressRootRecoverableErrors = null; // The most recent time we either committed a fallback, or when a fallback was
var workInProgressRootRecoverableErrors = null; // Tracks when an update occurs during the render phase.

var workInProgressRootDidIncludeRecursiveRenderUpdate = false; // Thacks when an update occurs during the commit phase. It's a separate
// variable from the one for renders because the commit phase may run
// concurrently to a render phase.

var didIncludeCommitPhaseUpdate = false; // The most recent time we either committed a fallback, or when a fallback was
// filled in with the resolved UI. This lets us throttle the appearance of new
// content as it streams in, to minimize jank.
// TODO: Think of a better name for this variable?
Expand Down Expand Up @@ -25813,6 +25821,7 @@ if (__DEV__) {
root,
workInProgressRootRecoverableErrors,
workInProgressTransitions,
workInProgressRootDidIncludeRecursiveRenderUpdate,
workInProgressDeferredLane
);
} else {
Expand Down Expand Up @@ -25846,6 +25855,7 @@ if (__DEV__) {
finishedWork,
workInProgressRootRecoverableErrors,
workInProgressTransitions,
workInProgressRootDidIncludeRecursiveRenderUpdate,
lanes,
workInProgressDeferredLane
),
Expand All @@ -25860,6 +25870,7 @@ if (__DEV__) {
finishedWork,
workInProgressRootRecoverableErrors,
workInProgressTransitions,
workInProgressRootDidIncludeRecursiveRenderUpdate,
lanes,
workInProgressDeferredLane
);
Expand All @@ -25871,6 +25882,7 @@ if (__DEV__) {
finishedWork,
recoverableErrors,
transitions,
didIncludeRenderPhaseUpdate,
lanes,
spawnedLane
) {
Expand All @@ -25895,14 +25907,26 @@ if (__DEV__) {
// us that it's ready. This will be canceled if we start work on the
// root again.
root.cancelPendingCommit = schedulePendingCommit(
commitRoot.bind(null, root, recoverableErrors, transitions)
commitRoot.bind(
null,
root,
recoverableErrors,
transitions,
didIncludeRenderPhaseUpdate
)
);
markRootSuspended(root, lanes, spawnedLane);
return;
}
} // Otherwise, commit immediately.

commitRoot(root, recoverableErrors, transitions, spawnedLane);
commitRoot(
root,
recoverableErrors,
transitions,
didIncludeRenderPhaseUpdate,
spawnedLane
);
}

function isRenderConsistentWithExternalStores(finishedWork) {
Expand Down Expand Up @@ -25965,13 +25989,49 @@ if (__DEV__) {
// eslint-disable-next-line no-unreachable

return true;
} // The extra indirections around markRootUpdated and markRootSuspended is
// needed to avoid a circular dependency between this module and
// ReactFiberLane. There's probably a better way to split up these modules and
// avoid this problem. Perhaps all the root-marking functions should move into
// the work loop.

function markRootUpdated(root, updatedLanes) {
markRootUpdated$1(root, updatedLanes);

if (enableInfiniteRenderLoopDetection) {
// Check for recursive updates
if (executionContext & RenderContext) {
workInProgressRootDidIncludeRecursiveRenderUpdate = true;
} else if (executionContext & CommitContext) {
didIncludeCommitPhaseUpdate = true;
}

throwIfInfiniteUpdateLoopDetected();
}
}

function markRootPinged(root, pingedLanes) {
markRootPinged$1(root, pingedLanes);

if (enableInfiniteRenderLoopDetection) {
// Check for recursive pings. Pings are conceptually different from updates in
// other contexts but we call it an "update" in this context because
// repeatedly pinging a suspended render can cause a recursive render loop.
// The relevant property is that it can result in a new render attempt
// being scheduled.
if (executionContext & RenderContext) {
workInProgressRootDidIncludeRecursiveRenderUpdate = true;
} else if (executionContext & CommitContext) {
didIncludeCommitPhaseUpdate = true;
}

throwIfInfiniteUpdateLoopDetected();
}
}

function markRootSuspended(root, suspendedLanes, spawnedLane) {
// When suspending, we should always exclude lanes that were pinged or (more
// rarely, since we try to avoid it) updated during the render phase.
// TODO: Lol maybe there's a better way to factor this besides this
// obnoxiously named function :)
suspendedLanes = removeLanes(
suspendedLanes,
workInProgressRootPingedLanes
Expand All @@ -25980,6 +26040,7 @@ if (__DEV__) {
suspendedLanes,
workInProgressRootInterleavedUpdatedLanes
);

markRootSuspended$1(root, suspendedLanes, spawnedLane);
} // This is the entry point for synchronous tasks that don't go
// through Scheduler
Expand Down Expand Up @@ -26054,6 +26115,7 @@ if (__DEV__) {
root,
workInProgressRootRecoverableErrors,
workInProgressTransitions,
workInProgressRootDidIncludeRecursiveRenderUpdate,
workInProgressDeferredLane
); // Before exiting, make sure there's a callback scheduled for the next
// pending level.
Expand Down Expand Up @@ -26183,7 +26245,8 @@ if (__DEV__) {
workInProgressRootPingedLanes = NoLanes;
workInProgressDeferredLane = NoLane;
workInProgressRootConcurrentErrors = null;
workInProgressRootRecoverableErrors = null; // Get the lanes that are entangled with whatever we're about to render. We
workInProgressRootRecoverableErrors = null;
workInProgressRootDidIncludeRecursiveRenderUpdate = false; // Get the lanes that are entangled with whatever we're about to render. We
// track these separately so we can distinguish the priority of the render
// task from the priority of the lanes it is entangled with. For example, a
// transition may not be allowed to finish unless it includes the Sync lane,
Expand Down Expand Up @@ -27239,7 +27302,13 @@ if (__DEV__) {
workInProgress = null;
}

function commitRoot(root, recoverableErrors, transitions, spawnedLane) {
function commitRoot(
root,
recoverableErrors,
transitions,
didIncludeRenderPhaseUpdate,
spawnedLane
) {
// TODO: This no longer makes any sense. We already wrap the mutation and
// layout phases. Should be able to remove.
var previousUpdateLanePriority = getCurrentUpdatePriority();
Expand All @@ -27252,6 +27321,7 @@ if (__DEV__) {
root,
recoverableErrors,
transitions,
didIncludeRenderPhaseUpdate,
previousUpdateLanePriority,
spawnedLane
);
Expand All @@ -27267,6 +27337,7 @@ if (__DEV__) {
root,
recoverableErrors,
transitions,
didIncludeRenderPhaseUpdate,
renderPriorityLevel,
spawnedLane
) {
Expand Down Expand Up @@ -27346,7 +27417,9 @@ if (__DEV__) {

var concurrentlyUpdatedLanes = getConcurrentlyUpdatedLanes();
remainingLanes = mergeLanes(remainingLanes, concurrentlyUpdatedLanes);
markRootFinished(root, remainingLanes, spawnedLane);
markRootFinished(root, remainingLanes, spawnedLane); // Reset this before firing side effects so we can detect recursive updates.

didIncludeCommitPhaseUpdate = false;

if (root === workInProgressRoot) {
// We can reset these now that they are finished.
Expand Down Expand Up @@ -27572,9 +27645,13 @@ if (__DEV__) {
// hydration is conceptually not an update.

if (
// Was the finished render the result of an update (not hydration)?
includesSomeLane(lanes, UpdateLanes) && // Did it schedule a sync update?
includesSomeLane(remainingLanes, SyncUpdateLanes)
// Check if there was a recursive update spawned by this render, in either
// the render phase or the commit phase. We track these explicitly because
// we can't infer from the remaining lanes alone.
(enableInfiniteRenderLoopDetection &&
(didIncludeRenderPhaseUpdate || didIncludeCommitPhaseUpdate)) || // Was the finished render the result of an update (not hydration)?
(includesSomeLane(lanes, UpdateLanes) && // Did it schedule a sync update?
includesSomeLane(remainingLanes, SyncUpdateLanes))
) {
{
markNestedUpdateScheduled();
Expand Down Expand Up @@ -28076,6 +28153,20 @@ if (__DEV__) {
nestedPassiveUpdateCount = 0;
rootWithNestedUpdates = null;
rootWithPassiveNestedUpdates = null;

if (enableInfiniteRenderLoopDetection) {
if (executionContext & RenderContext && workInProgressRoot !== null) {
// We're in the render phase. Disable the concurrent error recovery
// mechanism to ensure that the error we're about to throw gets handled.
// We need it to trigger the nearest error boundary so that the infinite
// update loop is broken.
workInProgressRoot.errorRecoveryDisabledLanes = mergeLanes(
workInProgressRoot.errorRecoveryDisabledLanes,
workInProgressRootRenderLanes
);
}
}

throw new Error(
"Maximum update depth exceeded. This can happen when a component " +
"repeatedly calls setState inside componentWillUpdate or " +
Expand Down
Loading

0 comments on commit e0e832e

Please sign in to comment.