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.

Sign up for GitHub

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

Jump to bottom

APP_INITIALIZER with HttpClient provoke memory leak in SSR projects #55396

Closed
dancornilov opened this issue Apr 18, 2024 · 9 comments · May be fixed by #55476
Closed

APP_INITIALIZER with HttpClient provoke memory leak in SSR projects #55396

dancornilov opened this issue Apr 18, 2024 · 9 comments · May be fixed by #55476
Labels
area: core Issues related to the framework runtime memory leak Issue related to a memory leak P4 A relatively minor issue that is not relevant to core functions state: has PR
Milestone
Backlog

Comments

@dancornilov
Copy link

Which @angular/* package(s) are the source of the bug?

common, core

Is this a regression?

Yes

Description

In my Angular 17 SSR project, I encountered a problem with memory consumption. Typically, this issue can arise due to unsubscription or intervals, but not this time.

After several weeks of debugging and refactoring everything that came to mind and could potentially cause this problem, I sought help from other friends, colleagues, and developers. Ultimately, after a full day of debugging, the bug was identified: the memory leak was caused by the HttpClient used in APP_INITIALIZER.

Right now, I've found a workaround to avoid this issue by creating a WrapperComponent which wraps all my routes, and I use a resolver to prevent HTTP requests in APP_INITIALIZER.

Please provide a link to a minimal reproduction of the bug

https://github.com/dancornilov/initializer-memory-leak

Please provide the exception or error you saw

No response

Please provide the environment you discovered this bug in (run ng version)

Angular CLI: 17.3.4
Node: 20.9.0
Package Manager: npm 10.1.0
OS: darwin arm64

Angular: 17.3.4
... animations, cdk, cli, common, compiler, compiler-cli, core
... forms, google-maps, material, platform-browser
... platform-browser-dynamic, platform-server, router
... service-worker, ssr

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1703.4
@angular-devkit/build-angular   17.3.4
@angular-devkit/core            17.3.4
@angular-devkit/schematics      17.3.4
@schematics/angular             17.3.4
rxjs                            7.8.1
typescript                      5.2.2
zone.js                         0.14.4

Anything else?

Please take a look in provided repository for minimal reproduction of the bug, in README is describe the full flow how to reproduce this issue.
@JeanMeche
Copy link
Member

JeanMeche commented Apr 18, 2024 •
edited

By any chance could you run the same tests without the fetch implementation ? (remove withFetch() to rely on the default XHR implementation) ?

@dancornilov
Copy link
Author

Hey @JeanMeche, I removed withFetch() but anyway problem still persist.

Screenshot 2024-04-19 at 12 46 13
  • green: Initial snapshot when app is only served
  • red: Snapshot when artillery finished his work
  • orange: Snapshot when I triggered manually GC

@alan-agius4 alan-agius4 added memory leak Issue related to a memory leak area: core Issues related to the framework runtime area: common/http labels Apr 19, 2024
@ngbot ngbot bot modified the milestone: needsTriage Apr 19, 2024
@JeanMeche
Copy link
Member

JeanMeche commented Apr 19, 2024 •
edited

I gave it a look and can provide 2 minimal repro :

This is linked to the TransferCache not being trashed by the GC.
What make this issue interesting is that the application builder repro does not reproduce the leak.

It looks like this is a dev-mode issue. When optimisation are enabled, I can't reproduce the leak.

@dancornilov Care to retry with optimization: true en your angular.json config ?

@dancornilov
Copy link
Author

I am not sure that I fully understand you, just to confirm:
In first repository you can reproduce it but in second this issue is not reproducible, am I right?

I retried my tests with optimization: true , yes this one helps to avoid this memory leak.

@JeanMeche
Copy link
Member

JeanMeche commented Apr 19, 2024 •
edited

I was mislead initially because the webpack builder didn't have the optimization enabled.
With optimization enabled, both don't reproduce the issue.

@dancornilov
Copy link
Author

I understand that this issue is most dev-only mode, but this is mistake or trade-of?

@JeanMeche
Copy link
Member

Its likely due to some debug/devtools related code. Those are only shipped on devbuilds. This why the Angular devtools only works on devbuilds for example.

@alan-agius4
Copy link
Contributor

@alxhub, suspects that the Injector Profiler is causing the leak.

angular/packages/core/src/render3/debug/framework_injector_profiler.ts

Line 56 in b1dffa4

class DIDebugData {

alan-agius4 added a commit to alan-agius4/angular that referenced this issue Apr 23, 2024
@alan-agius4
fix(core): use WeakRef to prevent object retention in WeakMap
28d83d0 
Although keys are not strongly referenced in a `WeakMap`, values are, potentially leading to data retention issues and improper garbage collection. By utilizing `WeakRef`, this problem can be mitigated effectively. Weak references allow the garbage collector to collect an object even if it is only weakly referenced. This can prevent memory leaks in the injector debugger profiler.

Closes angular#55396
@alan-agius4 alan-agius4 mentioned this issue Apr 23, 2024
Open
@AndrewKushnir AndrewKushnir added state: has PR P4 A relatively minor issue that is not relevant to core functions labels Apr 24, 2024
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Apr 24, 2024
alan-agius4 added a commit to alan-agius4/angular that referenced this issue Apr 26, 2024
@alan-agius4
fix(core): use WeakRef to prevent object retention in WeakMap
27ad5f5 
Although keys are not strongly referenced in a `WeakMap`, values are, potentially leading to data retention issues and improper garbage collection. By utilizing `WeakRef`, this problem can be mitigated effectively. Weak references allow the garbage collector to collect an object even if it is only weakly referenced. This can prevent memory leaks in the injector debugger profiler.

Closes angular#55396
@alan-agius4
Copy link
Contributor

Closed via #55530

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area: core Issues related to the framework runtime memory leak Issue related to a memory leak P4 A relatively minor issue that is not relevant to core functions state: has PR
Projects
None yet
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

fix(core): use WeakRef to prevent object retention in WeakMap alan-agius4/angular
4 participants
@JeanMeche @alan-agius4 @dancornilov @AndrewKushnir