Skip to content

fix(zone.js): remove abort listener once fetch is settled#57882

Closed
arturovt wants to merge 1 commit intoangular:mainfrom
arturovt:fix/zone-fetch-abort
Closed

fix(zone.js): remove abort listener once fetch is settled#57882
arturovt wants to merge 1 commit intoangular:mainfrom
arturovt:fix/zone-fetch-abort

Conversation

@arturovt
Copy link
Copy Markdown
Contributor

This commit updates the fetch patch for zone.js. Currently, we're attaching an abort event listener on the signal (when it's provided) and never removing it. We should be good citizens and remove event listeners whenever objects need to be properly collected. In Firefox, when saving a heap snapshot and running it through fxsnapshot, querying AbortSignal will print a so-called "CaptureMap" with a list of "lambdas," indicating that the signal is not garbage collected because of the event listener lambda function.

@arturovt arturovt marked this pull request as ready for review September 21, 2024 05:49
@pullapprove pullapprove Bot requested a review from JiaLiPassion September 21, 2024 05:49
@pkozlowski-opensource pkozlowski-opensource added the area: zones Issues related to zone.js label Sep 23, 2024
@ngbot ngbot Bot added this to the Backlog milestone Sep 23, 2024
@arturovt arturovt force-pushed the fix/zone-fetch-abort branch from 54d3e58 to 938e2e1 Compare September 24, 2024 11:44
Copy link
Copy Markdown
Contributor

@JiaLiPassion JiaLiPassion left a comment

Choose a reason for hiding this comment

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

LGTM!

@AndrewKushnir AndrewKushnir added target: patch This PR is targeted for the next patch release action: presubmit The PR is in need of a google3 presubmit labels Oct 4, 2024
@AndrewKushnir
Copy link
Copy Markdown
Contributor

Presubmit.

This commit updates the `fetch` patch for zone.js. Currently, we're attaching an
`abort` event listener on the signal (when it's provided) and never removing it.
We should be good citizens and remove event listeners whenever objects need to be
properly collected. In Firefox, when saving a heap snapshot and running it through
`fxsnapshot`, querying `AbortSignal` will print a so-called "CaptureMap" with a list
of "lambdas," indicating that the signal is not garbage collected because of the event
listener lambda function.
@arturovt arturovt force-pushed the fix/zone-fetch-abort branch from 938e2e1 to ccd6e48 Compare October 4, 2024 09:07
@AndrewKushnir AndrewKushnir added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed action: presubmit The PR is in need of a google3 presubmit labels Oct 4, 2024
@AndrewKushnir
Copy link
Copy Markdown
Contributor

Caretaker note: presubmit is "green" for this PR.

@AndrewKushnir AndrewKushnir added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Oct 7, 2024
@AndrewKushnir
Copy link
Copy Markdown
Contributor

This PR was merged into the repository by commit 6976349.

The changes were merged into the following branches: main, 18.2.x

AndrewKushnir pushed a commit that referenced this pull request Oct 7, 2024
This commit updates the `fetch` patch for zone.js. Currently, we're attaching an
`abort` event listener on the signal (when it's provided) and never removing it.
We should be good citizens and remove event listeners whenever objects need to be
properly collected. In Firefox, when saving a heap snapshot and running it through
`fxsnapshot`, querying `AbortSignal` will print a so-called "CaptureMap" with a list
of "lambdas," indicating that the signal is not garbage collected because of the event
listener lambda function.

PR Close #57882
@arturovt arturovt deleted the fix/zone-fetch-abort branch October 7, 2024 16:01
@angular-automatic-lock-bot
Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Nov 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews action: merge The PR is ready for merge by the caretaker area: zones Issues related to zone.js merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants