From 5a3a154cd862e7294158d2d086a0a1a29bdbe09d Mon Sep 17 00:00:00 2001 From: arturovt Date: Thu, 3 Dec 2020 01:24:29 +0200 Subject: [PATCH] fix(core): unsubscribe from the `onError` when the root view is removed (#39940) At the moment, when creating a root module, a subscription to the `onError` subject is also created. It captures the scope where `NgModuleRef` is created and prevents it from being garbage collected. Also note that this `NgModuleRef` has a reference to the root module instance (e.g. `AppModule`), which also prevents it from being GC'd. PR Close #39940 --- packages/core/src/application_ref.ts | 17 ++++++++++------ .../core/test/acceptance/bootstrap_spec.ts | 20 +++++++++++++++++-- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/packages/core/src/application_ref.ts b/packages/core/src/application_ref.ts index ace2cec25b..5f833bbdda 100644 --- a/packages/core/src/application_ref.ts +++ b/packages/core/src/application_ref.ts @@ -350,12 +350,17 @@ export class PlatformRef { if (!exceptionHandler) { throw new Error('No ErrorHandler. Is platform module (BrowserModule) included?'); } - moduleRef.onDestroy(() => remove(this._modules, moduleRef)); - ngZone!.runOutsideAngular(() => ngZone!.onError.subscribe({ - next: (error: any) => { - exceptionHandler.handleError(error); - } - })); + ngZone!.runOutsideAngular(() => { + const subscription = ngZone!.onError.subscribe({ + next: (error: any) => { + exceptionHandler.handleError(error); + } + }); + moduleRef.onDestroy(() => { + remove(this._modules, moduleRef); + subscription.unsubscribe(); + }); + }); return _callAndReportToErrorHandler(exceptionHandler, ngZone!, () => { const initStatus: ApplicationInitStatus = moduleRef.injector.get(ApplicationInitStatus); initStatus.runInitializers(); diff --git a/packages/core/test/acceptance/bootstrap_spec.ts b/packages/core/test/acceptance/bootstrap_spec.ts index 020470d1a8..edd8eaa43d 100644 --- a/packages/core/test/acceptance/bootstrap_spec.ts +++ b/packages/core/test/acceptance/bootstrap_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {ApplicationRef, COMPILER_OPTIONS, Component, destroyPlatform, NgModule, TestabilityRegistry, ViewEncapsulation} from '@angular/core'; +import {ApplicationRef, COMPILER_OPTIONS, Component, destroyPlatform, NgModule, NgZone, TestabilityRegistry, ViewEncapsulation} from '@angular/core'; import {expect} from '@angular/core/testing/src/testing_internal'; import {BrowserModule} from '@angular/platform-browser'; import {platformBrowserDynamic} from '@angular/platform-browser-dynamic'; @@ -227,6 +227,22 @@ describe('bootstrap', () => { })); }); + describe('PlatformRef cleanup', () => { + it('should unsubscribe from `onError` when Injector is destroyed', + withBody('', async () => { + const TestModule = createComponentAndModule(); + + const ngModuleRef = await platformBrowserDynamic().bootstrapModule(TestModule); + const ngZone = ngModuleRef.injector.get(NgZone); + + expect(ngZone.onError.observers.length).toBe(1); + + ngModuleRef.destroy(); + + expect(ngZone.onError.observers.length).toBe(0); + })); + }); + onlyInIvy('options cannot be changed in Ivy').describe('changing bootstrap options', () => { beforeEach(() => { spyOn(console, 'error'); @@ -365,4 +381,4 @@ export class MultipleSelectorsAppComponent { bootstrap: [MultipleSelectorsAppComponent], }) export class MultipleSelectorsAppModule { -} \ No newline at end of file +}