honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(core): memory leak if view container host view is destroyed while view ref is not (#40219) 

When we attach a `ViewRef` to a `ViewContainerRef`, we save a reference to the container
onto the `ViewRef` so that we can remove it when the ref is destroyed. The problem is
that if the container's `hostView` is destroyed first, the `ViewRef` has no way of knowing
that it should stop referencing the container.

These changes remove the leak by not saving a reference at all. Instead, when a `ViewRef`
is destroyed, we clean it up through the `LContainer` directly. We don't need to worry
about the case where the container is destroyed before the view, because containers
automatically clean up all of their views upon destruction.

Fixes #38648.

PR Close #40219
This commit is contained in:
Kristiyan Kostadinov 2020-12-21 13:07:50 +02:00 committed by Andrew Scott
parent fdbd3cae8a
commit 4f73820ad6
4 changed files with 119 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

65
goldens/circular-deps/packages.json
View File

@ -108,45 +108,6 @@
"packages/compiler/src/render3/view/styling_builder.ts",
"packages/compiler/src/render3/view/template.ts"
],
[
"packages/core/src/change_detection/change_detection.ts",
"packages/core/src/change_detection/change_detector_ref.ts",
"packages/core/src/render3/view_ref.ts",
"packages/core/src/linker/view_container_ref.ts",
"packages/core/src/linker/component_factory.ts"
],
[
"packages/core/src/change_detection/change_detection.ts",
"packages/core/src/change_detection/change_detector_ref.ts",
"packages/core/src/render3/view_ref.ts",
"packages/core/src/linker/view_container_ref.ts",
"packages/core/src/linker/ng_module_factory.ts",
"packages/core/src/linker/component_factory_resolver.ts",
"packages/core/src/linker/component_factory.ts"
],
[
"packages/core/src/change_detection/change_detection.ts",
"packages/core/src/change_detection/change_detector_ref.ts",
"packages/core/src/render3/view_ref.ts",
"packages/core/src/linker/view_container_ref.ts",
"packages/core/src/linker/template_ref.ts",
"packages/core/src/render3/instructions/shared.ts",
"packages/core/src/error_handler.ts",
"packages/core/src/errors.ts",
"packages/core/src/view/types.ts",
"packages/core/src/linker/component_factory.ts"
],
[
"packages/core/src/change_detection/change_detection.ts",
"packages/core/src/change_detection/change_detector_ref.ts",
"packages/core/src/render3/view_ref.ts",
"packages/core/src/linker/view_container_ref.ts",
"packages/core/src/render3/instructions/shared.ts",
"packages/core/src/error_handler.ts",
"packages/core/src/errors.ts",
"packages/core/src/view/types.ts",
"packages/core/src/linker/component_factory.ts"
],
[
"packages/core/src/change_detection/change_detection.ts",
"packages/core/src/change_detection/change_detector_ref.ts",
@ -164,8 +125,6 @@
[
"packages/core/src/change_detection/change_detector_ref.ts",
"packages/core/src/render3/view_ref.ts",
"packages/core/src/linker/view_container_ref.ts",
"packages/core/src/linker/template_ref.ts",
"packages/core/src/linker/view_ref.ts"
],
[
@ -205,18 +164,28 @@
"packages/core/src/errors.ts",
"packages/core/src/view/types.ts"
],
[
"packages/core/src/error_handler.ts",
"packages/core/src/errors.ts",
"packages/core/src/view/types.ts",
"packages/core/src/linker/template_ref.ts",
"packages/core/src/render3/instructions/shared.ts"
],
[
"packages/core/src/error_handler.ts",
"packages/core/src/errors.ts",
"packages/core/src/view/types.ts",
"packages/core/src/linker/view_container_ref.ts",
"packages/core/src/render3/instructions/shared.ts"
],
[
"packages/core/src/linker/component_factory_resolver.ts",
"packages/core/src/linker/component_factory.ts",
"packages/core/src/linker/ng_module_factory.ts"
],
[
"packages/core/src/linker/template_ref.ts",
"packages/core/src/render3/view_ref.ts",
"packages/core/src/linker/view_container_ref.ts"
],
[
"packages/core/src/linker/view_container_ref.ts",
"packages/core/src/render3/view_ref.ts"
"packages/core/src/linker/component_factory_resolver.ts",
"packages/core/src/linker/ng_module_factory.ts"
],
[
"packages/core/src/metadata/directives.ts",

2
packages/core/src/linker/view_container_ref.ts
View File

@ -307,7 +307,7 @@ const R3ViewContainerRef = class ViewContainerRef extends VE_ViewContainerRef {
addViewToContainer(tView, lContainer[T_HOST], renderer, lView, parentRNode, beforeNode);
}
(viewRef as R3ViewRef<any>).attachToViewContainerRef(this);
(viewRef as R3ViewRef<any>).attachToViewContainerRef();
addToArray(getOrCreateViewRefs(lContainer), adjustedIdx, viewRef);
return viewRef;

38
packages/core/src/render3/view_ref.ts
View File

@ -7,12 +7,15 @@
*/
import {ChangeDetectorRef as viewEngine_ChangeDetectorRef} from '../change_detection/change_detector_ref';
import {ViewContainerRef as viewEngine_ViewContainerRef} from '../linker/view_container_ref';
import {EmbeddedViewRef as viewEngine_EmbeddedViewRef, InternalViewRef as viewEngine_InternalViewRef, ViewRefTracker} from '../linker/view_ref';
import {removeFromArray} from '../util/array_utils';
import {assertEqual} from '../util/assert';
import {collectNativeNodes} from './collect_native_nodes';
import {checkNoChangesInRootView, checkNoChangesInternal, detectChangesInRootView, detectChangesInternal, markViewDirty, storeCleanupWithContext} from './instructions/shared';
import {CONTEXT, FLAGS, LView, LViewFlags, TVIEW} from './interfaces/view';
import {destroyLView, renderDetachView} from './node_manipulation';
import {CONTAINER_HEADER_OFFSET, VIEW_REFS} from './interfaces/container';
import {isLContainer} from './interfaces/type_checks';
import {CONTEXT, FLAGS, LView, LViewFlags, PARENT, TVIEW} from './interfaces/view';
import {destroyLView, detachView, renderDetachView} from './node_manipulation';
@ -24,7 +27,7 @@ export interface viewEngine_ChangeDetectorRef_interface extends viewEngine_Chang
export class ViewRef<T> implements viewEngine_EmbeddedViewRef<T>, viewEngine_InternalViewRef,
viewEngine_ChangeDetectorRef_interface {
private _appRef: ViewRefTracker|null = null;
private _viewContainerRef: viewEngine_ViewContainerRef|null = null;
private _attachedToViewContainer = false;
get rootNodes(): any[] {
const lView = this._lView;
@ -65,14 +68,21 @@ export class ViewRef<T> implements viewEngine_EmbeddedViewRef<T>, viewEngine_Int
destroy(): void {
if (this._appRef) {
this._appRef.detachView(this);
} else if (this._viewContainerRef) {
const index = this._viewContainerRef.indexOf(this);
if (index > -1) {
this._viewContainerRef.detach(index);
} else if (this._attachedToViewContainer) {
const parent = this._lView[PARENT];
if (isLContainer(parent)) {
const viewRefs = parent[VIEW_REFS] as ViewRef<unknown>[] | null;
const index = viewRefs ? viewRefs.indexOf(this) : -1;
if (index > -1) {
ngDevMode &&
assertEqual(
index, parent.indexOf(this._lView) - CONTAINER_HEADER_OFFSET,
'An attached view should be in the same position within its container as its ViewRef in the VIEW_REFS array.');
detachView(parent, index);
removeFromArray(viewRefs!, index);
}
}
this._viewContainerRef = null;
this._attachedToViewContainer = false;
}
destroyLView(this._lView[TVIEW], this._lView);
}
@ -271,11 +281,11 @@ export class ViewRef<T> implements viewEngine_EmbeddedViewRef<T>, viewEngine_Int
checkNoChangesInternal(this._lView[TVIEW], this._lView, this.context);
}
attachToViewContainerRef(vcRef: viewEngine_ViewContainerRef) {
attachToViewContainerRef() {
if (this._appRef) {
throw new Error('This view is already attached directly to the ApplicationRef!');
}
this._viewContainerRef = vcRef;
this._attachedToViewContainer = true;
}
detachFromAppRef() {
@ -284,7 +294,7 @@ export class ViewRef<T> implements viewEngine_EmbeddedViewRef<T>, viewEngine_Int
}
attachToAppRef(appRef: ViewRefTracker) {
if (this._viewContainerRef) {
if (this._attachedToViewContainer) {
throw new Error('This view is already attached to a ViewContainer!');
}
this._appRef = appRef;

78
packages/core/test/acceptance/view_ref_spec.ts
View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {ApplicationRef, ChangeDetectorRef, Component, ComponentFactoryResolver, ComponentRef, ElementRef, Injector, NgModule} from '@angular/core';
import {ApplicationRef, ChangeDetectorRef, Component, ComponentFactoryResolver, ComponentRef, ElementRef, EmbeddedViewRef, Injector, NgModule, TemplateRef, ViewChild, ViewContainerRef} from '@angular/core';
import {InternalViewRef} from '@angular/core/src/linker/view_ref';
import {TestBed} from '@angular/core/testing';
@ -72,4 +72,80 @@ describe('ViewRef', () => {
expect(called).toBe(true);
});
it('should remove view ref from view container when destroyed', () => {
@Component({template: ''})
class DynamicComponent {
constructor(public viewContainerRef: ViewContainerRef) {}
}
@Component({template: `<ng-template>Hello</ng-template>`})
class App {
@ViewChild(TemplateRef) templateRef!: TemplateRef<any>;
componentRef!: ComponentRef<DynamicComponent>;
viewRef!: EmbeddedViewRef<any>;
constructor(
private _viewContainerRef: ViewContainerRef,
private _componentFactoryResolver: ComponentFactoryResolver) {}
create() {
const factory = this._componentFactoryResolver.resolveComponentFactory(DynamicComponent);
this.viewRef = this.templateRef.createEmbeddedView(null);
this.componentRef = this._viewContainerRef.createComponent(factory);
this.componentRef.instance.viewContainerRef.insert(this.viewRef);
}
}
@NgModule({declarations: [App, DynamicComponent], entryComponents: [DynamicComponent]})
class MyTestModule {
}
TestBed.configureTestingModule({imports: [MyTestModule]});
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
fixture.componentInstance.create();
const viewContainerRef = fixture.componentInstance.componentRef.instance.viewContainerRef;
expect(viewContainerRef.length).toBe(1);
fixture.componentInstance.viewRef.destroy();
expect(viewContainerRef.length).toBe(0);
});
it('should mark a ViewRef as destroyed when the host view is destroyed', () => {
@Component({template: ''})
class DynamicComponent {
constructor(public viewContainerRef: ViewContainerRef) {}
}
@Component({template: `<ng-template>Hello</ng-template>`})
class App {
@ViewChild(TemplateRef) templateRef!: TemplateRef<any>;
componentRef!: ComponentRef<DynamicComponent>;
viewRef!: EmbeddedViewRef<any>;
constructor(
private _viewContainerRef: ViewContainerRef,
private _componentFactoryResolver: ComponentFactoryResolver) {}
create() {
const factory = this._componentFactoryResolver.resolveComponentFactory(DynamicComponent);
this.viewRef = this.templateRef.createEmbeddedView(null);
this.componentRef = this._viewContainerRef.createComponent(factory);
this.componentRef.instance.viewContainerRef.insert(this.viewRef);
}
}
@NgModule({declarations: [App, DynamicComponent], entryComponents: [DynamicComponent]})
class MyTestModule {
}
TestBed.configureTestingModule({imports: [MyTestModule]});
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
fixture.componentInstance.create();
const {componentRef, viewRef} = fixture.componentInstance;
expect(viewRef.destroyed).toBe(false);
componentRef.hostView.destroy();
expect(viewRef.destroyed).toBe(true);
});
});