From 4b029996fef2aa9107aabb0e52f8c47379beaa93 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Thu, 16 Apr 2026 13:41:22 -0700 Subject: [PATCH] [Fizz] add additional task reentrancy protections The prior fix for finishedTask reentrancy solved an observed failure. This change adds a bit of defensive bookeeping to protect against other theoretical reentrant task finishing that might fail in simlar ways but where we don't have a clear demonstration of the bug. --- packages/react-server/src/ReactFizzServer.js | 23 +++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index c1fc1531d3d9..73b3e435d189 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -4438,9 +4438,15 @@ function erroredTask( const boundaryRow = boundary.row; if (boundaryRow !== null) { // Unblock the SuspenseListRow that was blocked by this boundary. + // finishSuspenseListRow → unblockSuspenseListRow → finishedTask reenters + // and decrements allPendingTasks. Pin the counter above zero so those + // nested calls can't trip completeAll before this outer frame's own + // zero check at the end. + request.allPendingTasks++; if (--boundaryRow.pendingTasks === 0) { finishSuspenseListRow(request, boundaryRow); } + request.allPendingTasks--; } // Regardless of what happens next, this boundary won't be displayed, @@ -4955,20 +4961,21 @@ function finishedTask( hoistHoistables(boundaryRow.hoistables, boundary.contentState); } if (!isEligibleForOutlining(request, boundary)) { - // abortTaskSoft reenters finishedTask for each aborted task, which - // decrements allPendingTasks. Ensure that these reentrant finsihedTask - // calls do not call `completeAll` too early by forcing the task counter - // above zero for their duration. + // abortTaskSoft (below) and finishSuspenseListRow → unblockSuspenseListRow + // → finishedTask (further below) both reenter finishedTask and decrement + // allPendingTasks. Pin the counter above zero for the duration of these + // fan-outs so a nested finishedTask can't observe 0 and call completeAll + // before this outer call reaches its own zero check. request.allPendingTasks++; boundary.fallbackAbortableTasks.forEach(abortTaskSoft, request); boundary.fallbackAbortableTasks.clear(); - request.allPendingTasks--; if (boundaryRow !== null) { // If we aren't eligible for outlining, we don't have to wait until we flush it. if (--boundaryRow.pendingTasks === 0) { finishSuspenseListRow(request, boundaryRow); } } + request.allPendingTasks--; } if ( @@ -4994,11 +5001,17 @@ function finishedTask( boundaryRow.next, ); } + // finishSuspenseListRow → unblockSuspenseListRow → finishedTask reenters + // and decrements allPendingTasks. Pin the counter above zero so those + // nested calls can't trip completeAll before this outer frame's own + // zero check at the end. + request.allPendingTasks++; if (--boundaryRow.pendingTasks === 0) { // This is really unnecessary since we've already postponed the boundaries but // for pairity with other track+finish paths. We might end up using the hoisting. finishSuspenseListRow(request, boundaryRow); } + request.allPendingTasks--; } } } else {