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

Fix memory leaks #15397

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Fix memory leaks #15397

wants to merge 12 commits into from

Conversation

pmoleri
Copy link
Contributor

@pmoleri pmoleri commented Feb 24, 2025

Fix several memory leaks

Addressed issues:

  • Undestroyed Angular animations
  • Wrong addEventListeners using .bind() in-place.
  • .destroy() dynamic components created with createComponent()
  • .destroy() dynamic components created using ViewContainerRef.createComponent() but then detached from it.

Leak detection

Add leaks detection to the configureTestSuite().
If chrome is started with --js-flags="--expose-gc" it will track all created fixtures and perform a gc() at the end of the suite and it will report any leaked instances in memory.
Also, it's performing the moduleRef.destroy() for all created modules that was missing before.

The leak detection is opt-in for every suite because there are many existing issues and it may also report false positives for example:

  • When some timeout / requestAnimationFrame is pending to execute at the end of the test
  • When the reference is created beforeEach() but it's not cleaned up afterEach()

Not all leaks will be detected. For example a component retained by a root service, in the lifecycle of the test it will be disposed but in a real app lifecycle this should be considered a leak.

Additional information (check all that apply):

  • Bug fix
  • New functionality
  • Documentation
  • Demos
  • CI/CD

Checklist:

  • All relevant tags have been applied to this PR
  • This PR includes unit tests covering all the new code (test guidelines)
  • This PR includes API docs for newly added methods/properties (api docs guidelines)
  • This PR includes feature/README.MD updates for the feature docs
  • This PR includes general feature table updates in the root README.MD
  • This PR includes CHANGELOG.MD updates for newly added functionality
  • This PR contains breaking changes
  • This PR includes ng update migrations for the breaking changes (migrations guidelines)
  • This PR includes behavioral changes and the feature specification has been updated with them

@pmoleri pmoleri force-pushed the pmoleri/memory-leak-fixing branch 3 times, most recently from 1ee8fa3 to abd57c1 Compare February 26, 2025 15:15
@pmoleri pmoleri marked this pull request as ready for review February 26, 2025 18:49
@pmoleri pmoleri force-pushed the pmoleri/memory-leak-fixing branch from c19c10e to 2cadfa8 Compare February 26, 2025 19:00
@pmoleri pmoleri force-pushed the pmoleri/memory-leak-fixing branch from bf8cc4d to 082924f Compare February 27, 2025 13:35
@pmoleri pmoleri requested a review from kdinev February 27, 2025 14:39
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