Fix reentrancy bug

facebook · Apr 14, 2021 · ec2e4f2 · ec2e4f2
1 parent ea155e2
commit ec2e4f2
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 15 deletions.
diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
@@ -353,13 +353,16 @@ describe('ReactDOMFizzServer', () => {
 
     await act(async () => {
       const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable(
-        <Suspense fallback={<Text text="Loading A..." />}>
-          <>
-            <Text text="This will show A: " />
-            <div>
-              <AsyncText text="A" />
-            </div>
-          </>
+        // We use two nested boundaries to flush out coverage of an old reentrancy bug.
+        <Suspense fallback="Loading...">
+          <Suspense fallback={<Text text="Loading A..." />}>
+            <>
+              <Text text="This will show A: " />
+              <div>
+                <AsyncText text="A" />
+              </div>
+            </>
+          </Suspense>
         </Suspense>,
         writableA,
         {

diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js
@@ -1017,11 +1017,6 @@ function abortTask(task: Task): void {
   } else {
     boundary.pendingTasks--;
 
-    // If this boundary was still pending then we haven't already cancelled its fallbacks.
-    // We'll need to abort the fallbacks, which will also error that parent boundary.
-    boundary.fallbackAbortableTasks.forEach(abortTask, request);
-    boundary.fallbackAbortableTasks.clear();
-
     if (!boundary.forceClientRender) {
       boundary.forceClientRender = true;
       if (boundary.parentFlushed) {
@@ -1060,9 +1055,6 @@ function finishedTask(
       // This already errored.
     } else if (boundary.pendingTasks === 0) {
       // This must have been the last segment we were waiting on. This boundary is now complete.
-      // We can now cancel any pending task on the fallback since we won't need to show it anymore.
-      boundary.fallbackAbortableTasks.forEach(abortTaskSoft, request);
-      boundary.fallbackAbortableTasks.clear();
       if (segment.parentFlushed) {
         // Our parent segment already flushed, so we need to schedule this segment to be emitted.
         boundary.completedSegments.push(segment);
@@ -1072,6 +1064,11 @@ function finishedTask(
         // parent flushed, we need to schedule the boundary to be emitted.
         request.completedBoundaries.push(boundary);
       }
+      // We can now cancel any pending task on the fallback since we won't need to show it anymore.
+      // This needs to happen after we read the parentFlushed flags because aborting can finish
+      // work which can trigger user code, which can start flushing, which can change those flags.
+      boundary.fallbackAbortableTasks.forEach(abortTaskSoft, request);
+      boundary.fallbackAbortableTasks.clear();
     } else {
       if (segment.parentFlushed) {
         // Our parent already flushed, so we need to schedule this segment to be emitted.