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

NgModule Injector returning wrong providers in server application due to global state #25018

Closed
vikerman opened this issue Jul 22, 2018 · 1 comment
Closed

NgModule Injector returning wrong providers in server application due to global state #25018

vikerman opened this issue Jul 22, 2018 · 1 comment
Assignees
@vikerman
Labels
area: core Issues related to the framework runtime effort1: hours freq2: medium

Comments

@vikerman
Copy link
Contributor

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question
[ ] Other... Please describe:

Current behavior

When a NgModule is bootstrapped as part of different server requests in the same Node process, the provider returned by the NgModule injector might be complete wrong. This is because tree shakeable providers from one NgModule leak into the NgModule of the same type in the next request. This might result in the injector returning completely wrong provider.

The DEFINITION_CACHE is a global and shared across requests -
https://github.com/angular/angular/blob/master/packages/core/src/view/util.ts#L246

Resulting in a stale NgModuleDefinition here -
https://github.com/angular/angular/blob/master/packages/core/src/view/entrypoint.ts#L52

This is ok in the normal case since the providers list is fixed for a NgModule. But when there are tree shakeable providers the providers list is shared across requests resulting in wrong providers being returned for a given token because now two providers end up with the same index.

Expected behavior

Tree shakable Providers instances don't leak across requests.
@vikerman vikerman added severity3: broken area: core Issues related to the framework runtime labels Jul 22, 2018
@vikerman vikerman self-assigned this Jul 22, 2018
@ngbot ngbot bot added this to the needsTriage milestone Jul 22, 2018
@vikerman vikerman modified the milestones: needsTriage, 6.1 Jul 22, 2018
vikerman added a commit to vikerman/angular that referenced this issue Jul 22, 2018
@vikerman
fix(core): stop reusing provider definitions across NgModuleRef insta…
83986c0 
…nces

Fixes angular#25018.

Instantiating a NgModuleRef from NgModuleFactory reuses the NgModuleDefinition if it is already present. However the NgModuleDefinition has a providers array which modified when tree shakable providers are instantiated. This corrupts the provider definitions the next time the same factory is used to create a new NgModuleRef - Two provider definitions can end up with the same index anf the injector could potentially return a completely wrong object for a provider token.

This scenario is more likely on the server where the same NgModuleFactory is reused across requests.

The fix clones the cached NgModuleDefinition so that any tree shakable providers added later do not affect the cached copy.
@vikerman vikerman mentioned this issue Jul 22, 2018
Closed
3 tasks
vikerman added a commit to vikerman/angular that referenced this issue Jul 22, 2018
@vikerman
fix(core): stop reusing provider definitions across NgModuleRef insta…
0a4b5d0 
…nces

Fixes angular#25018.

Instantiating a NgModuleRef from NgModuleFactory reuses the NgModuleDefinition if it is already present. However the NgModuleDefinition has a providers array which modified when tree shakable providers are instantiated. This corrupts the provider definitions the next time the same factory is used to create a new NgModuleRef - Two provider definitions can end up with the same index anf the injector could potentially return a completely wrong object for a provider token.

This scenario is more likely on the server where the same NgModuleFactory is reused across requests.

The fix clones the cached NgModuleDefinition so that any tree shakable providers added later do not affect the cached copy.
vikerman added a commit to vikerman/angular that referenced this issue Jul 22, 2018
@vikerman
fix(core): stop reusing provider definitions across NgModuleRef insta…
3b91f1f 
…nces

Fixes angular#25018.

Instantiating a NgModuleRef from NgModuleFactory reuses the NgModuleDefinition if it is already present. However the NgModuleDefinition has a providers array which modified when tree shakable providers are instantiated. This corrupts the provider definitions the next time the same factory is used to create a new NgModuleRef - Two provider definitions can end up with the same index anf the injector could potentially return a completely wrong object for a provider token.

This scenario is more likely on the server where the same NgModuleFactory is reused across requests.

The fix clones the cached NgModuleDefinition so that any tree shakable providers added later do not affect the cached copy.
vikerman added a commit to vikerman/angular that referenced this issue Jul 22, 2018
@vikerman
fix(core): stop reusing provider definitions across NgModuleRef insta…
e807f5b 
…nces

Fixes angular#25018.

Instantiating a NgModuleRef from NgModuleFactory reuses the NgModuleDefinition if it is already present. However the NgModuleDefinition has a providers array which modified when tree shakable providers are instantiated. This corrupts the provider definitions the next time the same factory is used to create a new NgModuleRef - Two provider definitions can end up with the same index anf the injector could potentially return a completely wrong object for a provider token.

This scenario is more likely on the server where the same NgModuleFactory is reused across requests.

The fix clones the cached NgModuleDefinition so that any tree shakable providers added later do not affect the cached copy.
@vicb vicb closed this as completed in 6b859da Jul 23, 2018
vikerman added a commit to vikerman/angular that referenced this issue Jul 25, 2018
@vikerman
fix(core): stop reusing provider definitions across NgModuleRef insta…
de08306 
…nces

Fixes angular#25018.

Instantiating a NgModuleRef from NgModuleFactory reuses the NgModuleDefinition if it is already present. However the NgModuleDefinition has a providers array which modified when tree shakable providers are instantiated. This corrupts the provider definitions the next time the same factory is used to create a new NgModuleRef - Two provider definitions can end up with the same index and the injector could potentially return a completely wrong object for a provider token.

This scenario is more likely on the server where the same NgModuleFactory is reused across requests.

The fix clones the cached NgModuleDefinition so that any tree shakable providers added later do not affect the cached copy.

PR Close angular#25022
@vikerman vikerman mentioned this issue Jul 25, 2018
Closed
3 tasks
vicb pushed a commit that referenced this issue Jul 25, 2018
@vikerman @vicb
fix(core): stop reusing provider definitions across NgModuleRef insta…
0560751 
…nces (#25088)

Fixes #25018.

Instantiating a NgModuleRef from NgModuleFactory reuses the NgModuleDefinition if it is already present. However the NgModuleDefinition has a providers array which modified when tree shakable providers are instantiated. This corrupts the provider definitions the next time the same factory is used to create a new NgModuleRef - Two provider definitions can end up with the same index and the injector could potentially return a completely wrong object for a provider token.

This scenario is more likely on the server where the same NgModuleFactory is reused across requests.

The fix clones the cached NgModuleDefinition so that any tree shakable providers added later do not affect the cached copy.

PR Close #25022

PR Close #25088
@angular-automatic-lock-bot
Copy link

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 Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@vikerman vikerman

Labels
area: core Issues related to the framework runtime effort1: hours freq2: medium
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(core): stop reusing provider definitions across NgModuleRef insta… vikerman/angular
1 participant
@vikerman