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.

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

@@ -7,7 +7,7 @@
  * @noflow
  * @nolint
  * @preventMunge
- * @generated SignedSource<<bfa37b9235cbe10193422349b5fb18a5>>
+ * @generated SignedSource<<658af937953acc877e6c9c23bdafbf60>>
  */
 
 'use strict';
@@ -1584,7 +1584,7 @@ function createLaneMap(initial) {
 
  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.
@@ -1617,7 +1617,7 @@ function markRootSuspended$1(root, suspendedLanes) {
  lanes &= ~lane;
  }
 }
-function markRootPinged(root, pingedLanes) {
+function markRootPinged$1(root, pingedLanes) {
  root.pingedLanes |= root.suspendedLanes & pingedLanes;
 }
 function markRootFinished(root, remainingLanes) {
@@ -6004,6 +6004,21 @@ 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.
@@ -6032,11 +6047,49 @@ 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;
+ 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;
+ }
+
  var root = firstScheduledRoot;
 
  while (root !== null) {
@@ -19938,7 +19991,13 @@ 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; // 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?
@@ -20420,7 +20479,8 @@ function finishConcurrentRender(root, exitStatus, finishedWork, lanes) {
  commitRoot(
  root,
  workInProgressRootRecoverableErrors,
- workInProgressTransitions
+ workInProgressTransitions,
+ workInProgressRootDidIncludeRecursiveRenderUpdate
  );
  } else {
  if (includesOnlyRetries(lanes) && alwaysThrottleRetries) {
@@ -20450,6 +20510,7 @@ function finishConcurrentRender(root, exitStatus, finishedWork, lanes) {
  finishedWork,
  workInProgressRootRecoverableErrors,
  workInProgressTransitions,
+ workInProgressRootDidIncludeRecursiveRenderUpdate,
  lanes
  ),
  msUntilTimeout
@@ -20463,6 +20524,7 @@ function finishConcurrentRender(root, exitStatus, finishedWork, lanes) {
  finishedWork,
  workInProgressRootRecoverableErrors,
  workInProgressTransitions,
+ workInProgressRootDidIncludeRecursiveRenderUpdate,
  lanes
  );
  }
@@ -20473,6 +20535,7 @@ function commitRootWhenReady(
  finishedWork,
  recoverableErrors,
  transitions,
+ didIncludeRenderPhaseUpdate,
  lanes
 ) {
  // TODO: Combine retry throttling with Suspensey commits. Right now they run
@@ -20496,14 +20559,20 @@ 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)
+ commitRoot.bind(
+ null,
+ root,
+ recoverableErrors,
+ transitions,
+ didIncludeRenderPhaseUpdate
+ )
  );
  markRootSuspended(root, lanes);
  return;
  }
  } // Otherwise, commit immediately.
 
- commitRoot(root, recoverableErrors, transitions);
+ commitRoot(root, recoverableErrors, transitions, didIncludeRenderPhaseUpdate);
 }
 
 function isRenderConsistentWithExternalStores(finishedWork) {
@@ -20566,18 +20635,49 @@ 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
@@ -20648,7 +20748,8 @@ function performSyncWorkOnRoot(root) {
  commitRoot(
  root,
  workInProgressRootRecoverableErrors,
- workInProgressTransitions
+ workInProgressTransitions,
+ workInProgressRootDidIncludeRecursiveRenderUpdate
  ); // Before exiting, make sure there's a callback scheduled for the next
  // pending level.
 
@@ -20789,6 +20890,7 @@ function prepareFreshStack(root, lanes) {
  workInProgressRootPingedLanes = NoLanes;
  workInProgressRootConcurrentErrors = null;
  workInProgressRootRecoverableErrors = null;
+ workInProgressRootDidIncludeRecursiveRenderUpdate = false;
  finishQueueingConcurrentUpdates();
 
  {
@@ -21685,7 +21787,12 @@ function unwindUnitOfWork(unitOfWork) {
  workInProgress = null;
 }
 
-function commitRoot(root, recoverableErrors, transitions) {
+function commitRoot(
+ root,
+ recoverableErrors,
+ transitions,
+ didIncludeRenderPhaseUpdate
+) {
  // TODO: This no longer makes any sense. We already wrap the mutation and
  // layout phases. Should be able to remove.
  var previousUpdateLanePriority = getCurrentUpdatePriority();
@@ -21698,6 +21805,7 @@ function commitRoot(root, recoverableErrors, transitions) {
  root,
  recoverableErrors,
  transitions,
+ didIncludeRenderPhaseUpdate,
  previousUpdateLanePriority
  );
  } finally {
@@ -21712,6 +21820,7 @@ function commitRootImpl(
  root,
  recoverableErrors,
  transitions,
+ didIncludeRenderPhaseUpdate,
  renderPriorityLevel
 ) {
  do {
@@ -21767,7 +21876,9 @@ function commitRootImpl(
 
  var concurrentlyUpdatedLanes = getConcurrentlyUpdatedLanes();
  remainingLanes = mergeLanes(remainingLanes, concurrentlyUpdatedLanes);
- markRootFinished(root, remainingLanes);
+ markRootFinished(root, remainingLanes); // 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.
@@ -21948,7 +22059,18 @@ function commitRootImpl(
 
  remainingLanes = root.pendingLanes;
 
- if (includesSyncLane(remainingLanes)) {
+ 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)
+ ) {
  {
  markNestedUpdateScheduled();
  } // Count the number of times the root synchronously re-renders without
@@ -22386,6 +22508,18 @@ 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 " +
@@ -23857,7 +23991,7 @@ function createFiberRoot(
  return root;
 }
 
-var ReactVersion = "18.3.0-canary-80d9a4011-20230627";
+var ReactVersion = "18.3.0-canary-822386f25-20230627";
 
 // Might add PROFILE later.