Skip to content

Commit

Permalink
Revert "Fix: Detect infinite update loops caused by render phase upda…
Browse files Browse the repository at this point in the history 
…tes (#26625)" (#27027)

This reverts commit 822386f.

This broke a number of tests when synced internally. We'll need to
investigate the breakages before relanding this.

DiffTrain build for [7f362de](7f362de)
  • Loading branch information
@kassens
kassens committed Jun 30, 2023
1 parent 569cd7f commit 023fffc
Show file tree
Hide file tree
Showing 20 changed files with 460 additions and 1,912 deletions.
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
View file
Original file line number Diff line number Diff line change
@@ -1 +1 @@
fc801116c80b68f7ebdaf66ac77d5f2dcd9e50eb
7f362de1588d98438787d652941533e21f2f332d
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-dev.classic.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ if (
}
"use strict";

var ReactVersion = "18.3.0-www-classic-a3146e4f";
var ReactVersion = "18.3.0-www-classic-60aa7d44";

// ATTENTION
// When adding new symbols to this file,
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-dev.modern.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ if (
}
"use strict";

var ReactVersion = "18.3.0-www-modern-f95d3ddd";
var ReactVersion = "18.3.0-www-modern-fec65964";

// ATTENTION
// When adding new symbols to this file,
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-prod.modern.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -622,4 +622,4 @@ exports.useSyncExternalStore = function (
);
};
exports.useTransition = useTransition;
exports.version = "18.3.0-www-modern-0b35a427";
exports.version = "18.3.0-www-modern-e397df4a";
162 changes: 14 additions & 148 deletions compiled/facebook-www/ReactART-dev.classic.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
return self;
}

var ReactVersion = "18.3.0-www-classic-a18a6ba5";
var ReactVersion = "18.3.0-www-classic-9da53968";

var LegacyRoot = 0;
var ConcurrentRoot = 1;
Expand Down Expand Up @@ -2165,7 +2165,7 @@ function createLaneMap(initial) {

return laneMap;
}
function markRootUpdated$1(root, updateLane) {
function markRootUpdated(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 @@ -2198,7 +2198,7 @@ function markRootSuspended$1(root, suspendedLanes) {
lanes &= ~lane;
}
}
function markRootPinged$1(root, pingedLanes) {
function markRootPinged(root, pingedLanes) {
root.pingedLanes |= root.suspendedLanes & pingedLanes;
}
function markRootFinished(root, remainingLanes) {
Expand Down Expand Up @@ -7347,21 +7347,6 @@ function ensureRootIsScheduled(root) {
ReactCurrentActQueue$2.didScheduleLegacyUpdate = true;
}
}

function unscheduleAllRoots() {
// This is only done in a fatal error situation, as a last resort to prevent
// an infinite render loop.
var root = firstScheduledRoot;

while (root !== null) {
var next = root.next;
root.next = null;
root = next;
}

firstScheduledRoot = lastScheduledRoot = null;
}

function flushSyncWorkOnAllRoots() {
// This is allowed to be called synchronously, but the caller should check
// the execution context first.
Expand Down Expand Up @@ -7390,49 +7375,11 @@ function flushSyncWorkAcrossRoots_impl(onlyLegacy) {
var workInProgressRootRenderLanes = getWorkInProgressRootRenderLanes(); // There may or may not be synchronous work scheduled. Let's check.

var didPerformSomeWork;
var nestedUpdatePasses = 0;
var errors = null;
isFlushingWork = true;

do {
didPerformSomeWork = false; // This outer loop re-runs if performing sync work on a root spawns
// additional sync work. If it happens too many times, it's very likely
// caused by some sort of infinite update loop. We already have a loop guard
// in place that will trigger an error on the n+1th update, but it's
// possible for that error to get swallowed if the setState is called from
// an unexpected place, like during the render phase. So as an added
// precaution, we also use a guard here.
//
// Ideally, there should be no known way to trigger this synchronous loop.
// It's really just here as a safety net.
//
// This limit is slightly larger than the one that throws inside setState,
// because that one is preferable because it includes a componens stack.

if (++nestedUpdatePasses > 60) {
// This is a fatal error, so we'll unschedule all the roots.
unscheduleAllRoots(); // TODO: Change this error message to something different to distinguish
// it from the one that is thrown from setState. Those are less fatal
// because they usually will result in the bad component being unmounted,
// and an error boundary being triggered, rather than us having to
// forcibly stop the entire scheduler.

var infiniteUpdateError = new Error(
"Maximum update depth exceeded. This can happen when a component " +
"repeatedly calls setState inside componentWillUpdate or " +
"componentDidUpdate. React limits the number of nested updates to " +
"prevent infinite loops."
);

if (errors === null) {
errors = [infiniteUpdateError];
} else {
errors.push(infiniteUpdateError);
}

break;
}

didPerformSomeWork = false;
var root = firstScheduledRoot;

while (root !== null) {
Expand Down Expand Up @@ -23748,13 +23695,7 @@ var workInProgressRootPingedLanes = NoLanes; // Errors that are thrown during th
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; // 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
var workInProgressRootRecoverableErrors = null; // 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 @@ -24433,8 +24374,7 @@ function finishConcurrentRender(root, exitStatus, finishedWork, lanes) {
commitRoot(
root,
workInProgressRootRecoverableErrors,
workInProgressTransitions,
workInProgressRootDidIncludeRecursiveRenderUpdate
workInProgressTransitions
);
} else {
if (
Expand Down Expand Up @@ -24467,7 +24407,6 @@ function finishConcurrentRender(root, exitStatus, finishedWork, lanes) {
finishedWork,
workInProgressRootRecoverableErrors,
workInProgressTransitions,
workInProgressRootDidIncludeRecursiveRenderUpdate,
lanes
),
msUntilTimeout
Expand All @@ -24481,7 +24420,6 @@ function finishConcurrentRender(root, exitStatus, finishedWork, lanes) {
finishedWork,
workInProgressRootRecoverableErrors,
workInProgressTransitions,
workInProgressRootDidIncludeRecursiveRenderUpdate,
lanes
);
}
Expand All @@ -24492,7 +24430,6 @@ function commitRootWhenReady(
finishedWork,
recoverableErrors,
transitions,
didIncludeRenderPhaseUpdate,
lanes
) {
// TODO: Combine retry throttling with Suspensey commits. Right now they run
Expand All @@ -24516,20 +24453,14 @@ function commitRootWhenReady(
// 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,
didIncludeRenderPhaseUpdate
)
commitRoot.bind(null, root, recoverableErrors, transitions)
);
markRootSuspended(root, lanes);
return;
}
} // Otherwise, commit immediately.

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

function isRenderConsistentWithExternalStores(finishedWork) {
Expand Down Expand Up @@ -24592,49 +24523,18 @@ function isRenderConsistentWithExternalStores(finishedWork) {
// 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); // Check for recursive updates

if (executionContext & RenderContext) {
workInProgressRootDidIncludeRecursiveRenderUpdate = true;
} else if (executionContext & CommitContext) {
didIncludeCommitPhaseUpdate = true;
}

throwIfInfiniteUpdateLoopDetected();
}

function markRootPinged(root, pingedLanes) {
markRootPinged$1(root, pingedLanes); // 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) {
// 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);
suspendedLanes = removeLanes(
suspendedLanes,
workInProgressRootInterleavedUpdatedLanes
);

markRootSuspended$1(root, suspendedLanes);
} // This is the entry point for synchronous tasks that don't go
// through Scheduler
Expand Down Expand Up @@ -24705,8 +24605,7 @@ function performSyncWorkOnRoot(root) {
commitRoot(
root,
workInProgressRootRecoverableErrors,
workInProgressTransitions,
workInProgressRootDidIncludeRecursiveRenderUpdate
workInProgressTransitions
); // Before exiting, make sure there's a callback scheduled for the next
// pending level.

Expand Down Expand Up @@ -24832,7 +24731,6 @@ function prepareFreshStack(root, lanes) {
workInProgressRootPingedLanes = NoLanes;
workInProgressRootConcurrentErrors = null;
workInProgressRootRecoverableErrors = null;
workInProgressRootDidIncludeRecursiveRenderUpdate = false;
finishQueueingConcurrentUpdates();

{
Expand Down Expand Up @@ -25853,12 +25751,7 @@ function unwindUnitOfWork(unitOfWork) {
workInProgress = null;
}

function commitRoot(
root,
recoverableErrors,
transitions,
didIncludeRenderPhaseUpdate
) {
function commitRoot(root, recoverableErrors, transitions) {
// 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 @@ -25871,7 +25764,6 @@ function commitRoot(
root,
recoverableErrors,
transitions,
didIncludeRenderPhaseUpdate,
previousUpdateLanePriority
);
} finally {
Expand All @@ -25886,7 +25778,6 @@ function commitRootImpl(
root,
recoverableErrors,
transitions,
didIncludeRenderPhaseUpdate,
renderPriorityLevel
) {
do {
Expand Down Expand Up @@ -25962,9 +25853,7 @@ function commitRootImpl(

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

didIncludeCommitPhaseUpdate = false;
markRootFinished(root, remainingLanes);

if (root === workInProgressRoot) {
// We can reset these now that they are finished.
Expand Down Expand Up @@ -26183,18 +26072,7 @@ function commitRootImpl(

remainingLanes = root.pendingLanes;

if (
// 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.
didIncludeCommitPhaseUpdate ||
didIncludeRenderPhaseUpdate || // As an additional precaution, we also check if there's any remaining sync
// work. Theoretically this should be unreachable but if there's a mistake
// in React it helps to be overly defensive given how hard it is to debug
// those scenarios otherwise. This won't catch recursive async updates,
// though, which is why we check the flags above first.
includesSyncLane(remainingLanes)
) {
if (includesSyncLane(remainingLanes)) {
{
markNestedUpdateScheduled();
} // Count the number of times the root synchronously re-renders without
Expand Down Expand Up @@ -26689,18 +26567,6 @@ function throwIfInfiniteUpdateLoopDetected() {
nestedPassiveUpdateCount = 0;
rootWithNestedUpdates = null;
rootWithPassiveNestedUpdates = null;

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 023fffc

Please sign in to comment.