Skip to content

Add failing test to demonstrate interleaving issue with Suspense #16632

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

Jessidhia
Copy link
Contributor

@Jessidhia Jessidhia commented Sep 2, 2019

cc/ @acdlite, as suggested on this twitter thread https://twitter.com/acdlite/status/1167531547508953088

There's a race condition that can happen on both Concurrent and Batched Modes Suspense where it's possible for all pending thenables to settle between the commitRoot continuation being yielded and it actually getting invoked, as shouldYieldToHost might return true if it ran out of frame time budget during the last performUnitOfWork iteration. This causes all of the retryTimedOutBoundary -> scheduleCallbackForRoot invocations to find a pre-existing Immediate callback already queued, and so no notifications are scheduled at all.

This is not really something that can be simulated on the mock scheduler (or I didn't find out how to), so it was brute-forced by inserting an intermediate continuation between workLoop and commitRoot.

@sizebot
Copy link

sizebot commented Sep 2, 2019

No significant bundle size changes to report.

Generated by 🚫 dangerJS against 9f1bc35

@Jessidhia Jessidhia changed the title Add test to demonstrate interleaving issue with Suspense Add failing test to demonstrate interleaving issue with Suspense Sep 2, 2019
@stale
Copy link

stale bot commented Jan 10, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Jan 10, 2020
@stale
Copy link

stale bot commented Jan 17, 2020

Closing this pull request after a prolonged period of inactivity. If this issue is still present in the latest release, please ask for this pull request to be reopened. Thank you!

@stale stale bot closed this Jan 17, 2020
@gaearon gaearon reopened this Apr 1, 2020
@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Apr 1, 2020
@rickhanlonii rickhanlonii self-requested a review February 11, 2021 04:44
@sebmarkbage sebmarkbage deleted the branch facebook:master October 20, 2022 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants