fix(core): Fix possible infinite loop with markForCheck by partially reverting #54074
#54329
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…y reverting angular#54074 In some situations, calling `markForCheck` can result in an infinite loop in seemingly valid scenarios. When a transplanted view is inserted before its declaration, it gets refreshed in the retry loop of `detectChanges`. At this point, the `Dirty` flag has been cleared from all parents. Calling `markForCheck` marks the insertion tree up to the root `Dirty`. If the declaration is checked again as a result (i.e. because it has default change detection) and is reachable because its parent was marked `Dirty`, this can cause an infinite loop. The declaration is refreshed again, so the insertion is marked for refresh (again). We enter an infinite loop if the insertion tree always calls `markForCheck` for some reason (i.e. `{{createReplayObservable() | async}}`). While the case above does fall into an infinite loop, it also truly is a problem in the application. While it's not an infinite synchronous loop, the declaration and insertion are infinitely dirty and will be refreshed on every change detection round. Usually `markForCheck` does not have this problem because the `Dirty` flag is not cleared until the very end of change detection. However, if the view did not already have the `Dirty` flag set, it is never cleared because we never entered view refresh. One solution to this problem could be to clear the `Dirty` flag even after skipping view refresh but traversing to children.
atscott
force-pushed
the
transplantedViewsAreProblematic
branch
from
c3f2ece
to
3b27313
Compare
atscott
added
area: core
atscott
added
the
regression
jessicajaniuk
pushed a commit
that referenced
this pull request
Feb 8, 2024
…y reverting #54074 (#54329) In some situations, calling `markForCheck` can result in an infinite loop in seemingly valid scenarios. When a transplanted view is inserted before its declaration, it gets refreshed in the retry loop of `detectChanges`. At this point, the `Dirty` flag has been cleared from all parents. Calling `markForCheck` marks the insertion tree up to the root `Dirty`. If the declaration is checked again as a result (i.e. because it has default change detection) and is reachable because its parent was marked `Dirty`, this can cause an infinite loop. The declaration is refreshed again, so the insertion is marked for refresh (again). We enter an infinite loop if the insertion tree always calls `markForCheck` for some reason (i.e. `{{createReplayObservable() | async}}`). While the case above does fall into an infinite loop, it also truly is a problem in the application. While it's not an infinite synchronous loop, the declaration and insertion are infinitely dirty and will be refreshed on every change detection round. Usually `markForCheck` does not have this problem because the `Dirty` flag is not cleared until the very end of change detection. However, if the view did not already have the `Dirty` flag set, it is never cleared because we never entered view refresh. One solution to this problem could be to clear the `Dirty` flag even after skipping view refresh but traversing to children. PR Close #54329
|
This PR was merged into the repository by commit 898a532. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In some situations, calling
markForCheckcan result in an infinite loop in seemingly valid scenarios. When a transplanted view is inserted before its declaration, it gets refreshed in the retry loop ofdetectChanges. At this point, theDirtyflag has been cleared from all parents. CallingmarkForCheckmarks the insertion tree up to the rootDirty. If the declaration is checked again as a result (i.e. because it has default change detection) and is reachable because its parent was markedDirty, this can cause an infinite loop. The declaration is refreshed again, so the insertion is marked for refresh (again). We enter an infinite loop if the insertion tree always callsmarkForCheckfor some reason (i.e.{{createReplayObservable() | async}}).While the case above does fall into an infinite loop, it also truly is a problem in the application. While it's not an infinite synchronous loop, the declaration and insertion are infinitely dirty and will be refreshed on every change detection round.
Usually
markForCheckdoes not have this problem because theDirtyflag is not cleared until the very end of change detection. However, if the view did not already have theDirtyflag set, it is never cleared because we never entered view refresh. One solution to this problem could be to clear theDirtyflag even after skipping view refresh but traversing to children.