KEMBAR78
[Fizz] Avoid hanging when suspending after aborting while rendering by gnoff · Pull Request #34192 · facebook/react · GitHub
Skip to content

Conversation

gnoff
Copy link
Collaborator

@gnoff gnoff commented Aug 12, 2025

This fixes an edge case where you abort the render while rendering a component that ends up Suspending. It technically only applied if you were deep enough to be inside renderNode and was not susceptible to hanging if the abort + suspending component was being tried inside retryRenderTask/retryReplaytask.

The fix is to preempt the thenable checks in renderNode and check if the request is aborting and if so just bubble up to the task handler.

The reason this hung before is a new task would get scheduled after we had aborted every other task (minus the currently rendering one). This led to a situation where the task count would not hit zero.

@gnoff gnoff requested a review from sebmarkbage August 12, 2025 22:18
@meta-cla meta-cla bot added the CLA Signed label Aug 12, 2025
@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label Aug 12, 2025
@react-sizebot
Copy link

react-sizebot commented Aug 12, 2025

Comparing: 0422a00...f1589a3

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.68 kB 6.68 kB = 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 530.16 kB 530.16 kB = 93.39 kB 93.39 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 654.53 kB 654.53 kB = 115.11 kB 115.11 kB
facebook-www/ReactDOM-prod.classic.js = 674.30 kB 674.30 kB = 118.29 kB 118.29 kB
facebook-www/ReactDOM-prod.modern.js = 664.73 kB 664.73 kB = 116.64 kB 116.64 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against f1589a3

@gnoff gnoff force-pushed the fix-fizz-suspend-after-abort branch from 6d7133b to a7a9639 Compare August 12, 2025 22:35
This fixes an edge case where you abort the render while rendering a component that ends up Suspending. It technically only applied if you were deep enough to be inside `renderNode` and was not susceptible to hanging if the abort + suspending component was being tried inside retryRenderTask/retryReplaytask.

The fix is to preempt the thenable checks in renderNode and check if the request is aborting and if so just bubble up to the task handler.

The reason this hung before is a new task would get scheduled after we had aborted every other task (minus the currently rendering one). This led to a situation where the task count would not hit zero.
@gnoff gnoff force-pushed the fix-fizz-suspend-after-abort branch from a7a9639 to f1589a3 Compare August 12, 2025 22:44
Copy link
Collaborator

@sebmarkbage sebmarkbage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ve been worried about the new task scheduled after abort coming up.

@gnoff gnoff merged commit 9baecbf into facebook:main Aug 12, 2025
241 checks passed
@gnoff gnoff deleted the fix-fizz-suspend-after-abort branch August 12, 2025 23:46
github-actions bot pushed a commit that referenced this pull request Aug 12, 2025
…34192)

This fixes an edge case where you abort the render while rendering a
component that ends up Suspending. It technically only applied if you
were deep enough to be inside `renderNode` and was not susceptible to
hanging if the abort + suspending component was being tried inside
retryRenderTask/retryReplaytask.

The fix is to preempt the thenable checks in renderNode and check if the
request is aborting and if so just bubble up to the task handler.

The reason this hung before is a new task would get scheduled after we
had aborted every other task (minus the currently rendering one). This
led to a situation where the task count would not hit zero.

DiffTrain build for [9baecbf](9baecbf)
gnoff pushed a commit to vercel/next.js that referenced this pull request Aug 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed React Core Team Opened by a member of the React Core Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants