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 commit 7f362de.
  • Loading branch information
@kassens
kassens committed Jun 30, 2023
1 parent f8f734e commit b2dc5de
Show file tree
Hide file tree
Showing 13 changed files with 227 additions and 899 deletions.
164 changes: 15 additions & 149 deletions ...ook-fbsource/xplat/js/RKJSModules/vendor/react-test-renderer/cjs/ReactTestRenderer-dev.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* @noflow
* @nolint
* @preventMunge
* @generated SignedSource<<b3edba4ee6aa87997395f161d846a4b0>>
* @generated SignedSource<<adf921170475fb87731c699acf7c0c8f>>
*/

'use strict';
Expand Down Expand Up @@ -1584,7 +1584,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 @@ -1617,7 +1617,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 @@ -6031,21 +6031,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 @@ -6074,49 +6059,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 @@ -20018,13 +19965,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 @@ -20506,8 +20447,7 @@ function finishConcurrentRender(root, exitStatus, finishedWork, lanes) {
commitRoot(
root,
workInProgressRootRecoverableErrors,
workInProgressTransitions,
workInProgressRootDidIncludeRecursiveRenderUpdate
workInProgressTransitions
);
} else {
if (includesOnlyRetries(lanes) && alwaysThrottleRetries) {
Expand Down Expand Up @@ -20537,7 +20477,6 @@ function finishConcurrentRender(root, exitStatus, finishedWork, lanes) {
finishedWork,
workInProgressRootRecoverableErrors,
workInProgressTransitions,
workInProgressRootDidIncludeRecursiveRenderUpdate,
lanes
),
msUntilTimeout
Expand All @@ -20551,7 +20490,6 @@ function finishConcurrentRender(root, exitStatus, finishedWork, lanes) {
finishedWork,
workInProgressRootRecoverableErrors,
workInProgressTransitions,
workInProgressRootDidIncludeRecursiveRenderUpdate,
lanes
);
}
Expand All @@ -20562,7 +20500,6 @@ function commitRootWhenReady(
finishedWork,
recoverableErrors,
transitions,
didIncludeRenderPhaseUpdate,
lanes
) {
// TODO: Combine retry throttling with Suspensey commits. Right now they run
Expand All @@ -20586,20 +20523,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 @@ -20662,49 +20593,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 @@ -20775,8 +20675,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 @@ -20917,7 +20816,6 @@ function prepareFreshStack(root, lanes) {
workInProgressRootPingedLanes = NoLanes;
workInProgressRootConcurrentErrors = null;
workInProgressRootRecoverableErrors = null;
workInProgressRootDidIncludeRecursiveRenderUpdate = false;
finishQueueingConcurrentUpdates();

{
Expand Down Expand Up @@ -21832,12 +21730,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 @@ -21850,7 +21743,6 @@ function commitRoot(
root,
recoverableErrors,
transitions,
didIncludeRenderPhaseUpdate,
previousUpdateLanePriority
);
} finally {
Expand All @@ -21865,7 +21757,6 @@ function commitRootImpl(
root,
recoverableErrors,
transitions,
didIncludeRenderPhaseUpdate,
renderPriorityLevel
) {
do {
Expand Down Expand Up @@ -21921,9 +21812,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 @@ -22104,18 +21993,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 @@ -22553,18 +22431,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 Expand Up @@ -24037,7 +23903,7 @@ function createFiberRoot(
return root;
}

var ReactVersion = "18.3.0-canary-fc801116c-20230629";
var ReactVersion = "18.3.0-canary-7f362de15-20230630";

// Might add PROFILE later.

Expand Down
Loading

0 comments on commit b2dc5de

Please sign in to comment.