Skip to content

fix(core): cleanup _ejsa when app is destroyed#59492

Closed
arturovt wants to merge 1 commit intoangular:mainfrom
arturovt:fix/core-add-remove-event-info-method
Closed

fix(core): cleanup _ejsa when app is destroyed#59492
arturovt wants to merge 1 commit intoangular:mainfrom
arturovt:fix/core-add-remove-event-info-method

Conversation

@arturovt
Copy link
Copy Markdown
Contributor

@arturovt arturovt commented Jan 12, 2025

In this commit, we delete _ejsa when the app is destroyed, ensuring that no elements are still captured in the global list and are not prevented from being garbage collected.

@angular-robot angular-robot bot added the area: core Issues related to the framework runtime label Jan 12, 2025
@ngbot ngbot bot added this to the Backlog milestone Jan 12, 2025
@tbondwilkinson
Copy link
Copy Markdown
Contributor

Generally speaking the EarlyJsactionData is cleared as soon as its used, so there would be no references to q left. It's unclear where you're seeing a memory leak.

@arturovt
Copy link
Copy Markdown
Contributor Author

Generally speaking the EarlyJsactionData is cleared as soon as its used, so there would be no references to q left. It's unclear where you're seeing a memory leak.

#59261

@tbondwilkinson
Copy link
Copy Markdown
Contributor

_ejsa should be deleted when the app is destroyed - that's the proper fix for this, I suspect

@arturovt arturovt force-pushed the fix/core-add-remove-event-info-method branch from c8ca84b to 301d77a Compare January 14, 2025 21:06
@arturovt arturovt changed the title fix(core): add remove event info handler fix(core): cleanup _ejsa when app is destroyed Jan 14, 2025
@arturovt arturovt changed the title fix(core): cleanup _ejsa when app is destroyed fix(core): cleanup _ejsa when app is destroyed Jan 14, 2025
@arturovt arturovt force-pushed the fix/core-add-remove-event-info-method branch from 301d77a to 9c72e21 Compare January 14, 2025 21:07
@thePunderWoman
Copy link
Copy Markdown
Contributor

@arturovt Can you add tests for this so we don't regress in the future?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe the method that's used below is:

clearAppScopedEarlyEventContract(appId);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reused that function.

@arturovt arturovt force-pushed the fix/core-add-remove-event-info-method branch from 9c72e21 to beaad8d Compare January 14, 2025 21:36
In this commit, we delete `_ejsa` when the app is destroyed, ensuring that no elements are still captured in the global list and are not prevented from being garbage collected.
@arturovt arturovt force-pushed the fix/core-add-remove-event-info-method branch from beaad8d to 08437e5 Compare January 14, 2025 21:37
@arturovt arturovt marked this pull request as ready for review January 15, 2025 08:58
@pullapprove pullapprove bot requested a review from thePunderWoman January 15, 2025 08:58
Copy link
Copy Markdown
Contributor

@thePunderWoman thePunderWoman left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@thePunderWoman thePunderWoman added target: patch This PR is targeted for the next patch release action: merge The PR is ready for merge by the caretaker labels Jan 16, 2025
@AndrewKushnir
Copy link
Copy Markdown
Contributor

This PR was merged into the repository by commit f6e9776.

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

AndrewKushnir pushed a commit that referenced this pull request Jan 16, 2025
In this commit, we delete `_ejsa` when the app is destroyed, ensuring that no elements are still captured in the global list and are not prevented from being garbage collected.

PR Close #59492
@arturovt arturovt deleted the fix/core-add-remove-event-info-method branch January 16, 2025 22:15
PrajaktaB27 pushed a commit to PrajaktaB27/angular that referenced this pull request Feb 7, 2025
In this commit, we delete `_ejsa` when the app is destroyed, ensuring that no elements are still captured in the global list and are not prevented from being garbage collected.

PR Close angular#59492
@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 Feb 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime 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