Skip to content

Commit 4b073f4

Browse files
authored
[Fizz] add additional task reentrancy protections (#36291)
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.
1 parent f6fe427 commit 4b073f4

1 file changed

Lines changed: 18 additions & 5 deletions

File tree

packages/react-server/src/ReactFizzServer.js

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4438,9 +4438,15 @@ function erroredTask(
44384438
const boundaryRow = boundary.row;
44394439
if (boundaryRow !== null) {
44404440
// Unblock the SuspenseListRow that was blocked by this boundary.
4441+
// finishSuspenseListRow → unblockSuspenseListRow → finishedTask reenters
4442+
// and decrements allPendingTasks. Pin the counter above zero so those
4443+
// nested calls can't trip completeAll before this outer frame's own
4444+
// zero check at the end.
4445+
request.allPendingTasks++;
44414446
if (--boundaryRow.pendingTasks === 0) {
44424447
finishSuspenseListRow(request, boundaryRow);
44434448
}
4449+
request.allPendingTasks--;
44444450
}
44454451

44464452
// Regardless of what happens next, this boundary won't be displayed,
@@ -4955,20 +4961,21 @@ function finishedTask(
49554961
hoistHoistables(boundaryRow.hoistables, boundary.contentState);
49564962
}
49574963
if (!isEligibleForOutlining(request, boundary)) {
4958-
// abortTaskSoft reenters finishedTask for each aborted task, which
4959-
// decrements allPendingTasks. Ensure that these reentrant finsihedTask
4960-
// calls do not call `completeAll` too early by forcing the task counter
4961-
// above zero for their duration.
4964+
// abortTaskSoft (below) and finishSuspenseListRow → unblockSuspenseListRow
4965+
// → finishedTask (further below) both reenter finishedTask and decrement
4966+
// allPendingTasks. Pin the counter above zero for the duration of these
4967+
// fan-outs so a nested finishedTask can't observe 0 and call completeAll
4968+
// before this outer call reaches its own zero check.
49624969
request.allPendingTasks++;
49634970
boundary.fallbackAbortableTasks.forEach(abortTaskSoft, request);
49644971
boundary.fallbackAbortableTasks.clear();
4965-
request.allPendingTasks--;
49664972
if (boundaryRow !== null) {
49674973
// If we aren't eligible for outlining, we don't have to wait until we flush it.
49684974
if (--boundaryRow.pendingTasks === 0) {
49694975
finishSuspenseListRow(request, boundaryRow);
49704976
}
49714977
}
4978+
request.allPendingTasks--;
49724979
}
49734980

49744981
if (
@@ -4994,11 +5001,17 @@ function finishedTask(
49945001
boundaryRow.next,
49955002
);
49965003
}
5004+
// finishSuspenseListRow → unblockSuspenseListRow → finishedTask reenters
5005+
// and decrements allPendingTasks. Pin the counter above zero so those
5006+
// nested calls can't trip completeAll before this outer frame's own
5007+
// zero check at the end.
5008+
request.allPendingTasks++;
49975009
if (--boundaryRow.pendingTasks === 0) {
49985010
// This is really unnecessary since we've already postponed the boundaries but
49995011
// for pairity with other track+finish paths. We might end up using the hoisting.
50005012
finishSuspenseListRow(request, boundaryRow);
50015013
}
5014+
request.allPendingTasks--;
50025015
}
50035016
}
50045017
} else {

0 commit comments

Comments
 (0)