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 [822386f252fd1f0e949efa904a1ed790133329f7](facebook/react@822386f)

Author: jerrydev0927

compiled/facebook-www/REVISION

@@ -1 +1 @@
-80d9a40114bb43c07d021e8254790852f450bd2b
+822386f252fd1f0e949efa904a1ed790133329f7

compiled/facebook-www/React-dev.classic.js

@@ -27,7 +27,7 @@ if (
 }
           "use strict";
 
-var ReactVersion = "18.3.0-www-classic-70d18c58";
+var ReactVersion = "18.3.0-www-classic-7bcc71be";
 
 // ATTENTION
 // When adding new symbols to this file,

compiled/facebook-www/React-dev.modern.js

@@ -27,7 +27,7 @@ if (
 }
           "use strict";
 
-var ReactVersion = "18.3.0-www-modern-6b726e24";
+var ReactVersion = "18.3.0-www-modern-fbda1d19";
 
 // ATTENTION
 // When adding new symbols to this file,

compiled/facebook-www/React-prod.modern.js

@@ -622,4 +622,4 @@ exports.useSyncExternalStore = function (
   );
 };
 exports.useTransition = useTransition;
-exports.version = "18.3.0-www-modern-ad08fdeb";
+exports.version = "18.3.0-www-modern-ead87ff5";

compiled/facebook-www/ReactART-dev.classic.js

@@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
   return self;
 }
 
-var ReactVersion = "18.3.0-www-classic-af595a64";
+var ReactVersion = "18.3.0-www-classic-4705a186";
 
 var LegacyRoot = 0;
 var ConcurrentRoot = 1;
@@ -2165,7 +2165,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.
@@ -2198,7 +2198,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) {
@@ -7320,6 +7320,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.
@@ -7348,11 +7363,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) {
@@ -23668,7 +23721,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?
@@ -24347,7 +24406,8 @@ function finishConcurrentRender(root, exitStatus, finishedWork, lanes) {
     commitRoot(
       root,
       workInProgressRootRecoverableErrors,
-      workInProgressTransitions
+      workInProgressTransitions,
+      workInProgressRootDidIncludeRecursiveRenderUpdate
     );
   } else {
     if (
@@ -24380,6 +24440,7 @@ function finishConcurrentRender(root, exitStatus, finishedWork, lanes) {
             finishedWork,
             workInProgressRootRecoverableErrors,
             workInProgressTransitions,
+            workInProgressRootDidIncludeRecursiveRenderUpdate,
             lanes
           ),
           msUntilTimeout
@@ -24393,6 +24454,7 @@ function finishConcurrentRender(root, exitStatus, finishedWork, lanes) {
       finishedWork,
       workInProgressRootRecoverableErrors,
       workInProgressTransitions,
+      workInProgressRootDidIncludeRecursiveRenderUpdate,
       lanes
     );
   }
@@ -24403,6 +24465,7 @@ function commitRootWhenReady(
   finishedWork,
   recoverableErrors,
   transitions,
+  didIncludeRenderPhaseUpdate,
   lanes
 ) {
   // TODO: Combine retry throttling with Suspensey commits. Right now they run
@@ -24426,14 +24489,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) {
@@ -24496,18 +24565,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
@@ -24578,7 +24678,8 @@ function performSyncWorkOnRoot(root) {
   commitRoot(
     root,
     workInProgressRootRecoverableErrors,
-    workInProgressTransitions
+    workInProgressTransitions,
+    workInProgressRootDidIncludeRecursiveRenderUpdate
   ); // Before exiting, make sure there's a callback scheduled for the next
   // pending level.
 
@@ -24704,6 +24805,7 @@ function prepareFreshStack(root, lanes) {
   workInProgressRootPingedLanes = NoLanes;
   workInProgressRootConcurrentErrors = null;
   workInProgressRootRecoverableErrors = null;
+  workInProgressRootDidIncludeRecursiveRenderUpdate = false;
   finishQueueingConcurrentUpdates();
 
   {
@@ -25706,7 +25808,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();
@@ -25719,6 +25826,7 @@ function commitRoot(root, recoverableErrors, transitions) {
       root,
       recoverableErrors,
       transitions,
+      didIncludeRenderPhaseUpdate,
       previousUpdateLanePriority
     );
   } finally {
@@ -25733,6 +25841,7 @@ function commitRootImpl(
   root,
   recoverableErrors,
   transitions,
+  didIncludeRenderPhaseUpdate,
   renderPriorityLevel
 ) {
   do {
@@ -25808,7 +25917,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.
@@ -26027,7 +26138,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
@@ -26522,6 +26644,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 " +