Skip to content
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

Track nested updates per root #10574

Merged
merged 1 commit into from
Aug 31, 2017

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Aug 30, 2017

We track nested updates to simulate a stack overflow error and prevent infinite loops. Every time we commit a tree, we increment a counter. This works if you only have one tree, but if you update many separate trees, it creates a false negative.

The fix is to reset the counter whenever we switch trees.

@@ -25,6 +25,8 @@ export type FiberRoot = {
isScheduled: boolean,
// The work schedule is a linked list.
nextScheduledRoot: FiberRoot | null,
// Track nested commits, to prevent an infinite loop.
nestedUpdateCount: number,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Needs to be added to createFiberRoot below to prevent deopting.

(Maybe we should add a preventExtensions here like we have in createFiber

Object.preventExtensions(this);
.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Flow also caught this error, I just forgot to run it

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.

This has a bug in it. If two roots update every unit of work, in the same order. Then the second one will never reset and eventually throw.

@acdlite acdlite force-pushed the tracknestedupdatesperroot branch 3 times, most recently from e1bda91 to 4e6a257 Compare August 30, 2017 23:19
@acdlite
Copy link
Collaborator Author

acdlite commented Aug 30, 2017

@sebmarkbage Updated

We track nested updates to simulate a stack overflow error and prevent
infinite loops. Every time we commit a tree, we increment a counter.
This works if you only have one tree, but if you update many separate
trees, it creates a false negative.

The fix is to reset the counter whenever we switch trees.
@acdlite acdlite merged commit a6e34cc into facebook:master Aug 31, 2017
@bvaughn bvaughn mentioned this pull request Sep 7, 2017
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.

4 participants