Skip to content

Commit 0d957c2

Browse files
PlatonnPlatonn
committed
fix(platform-server): destroy PlatformRef when error happens during the bootstrap() phase
The `bootstrap()` phase might fail e.g. due to an rejected promise in some `APP_INIIALIZER`. If `PlatformRef` is not destroyed, then the main app's injector is not destroyed and therefore `ngOnDestroy` hooks of singleton services is not called on the end (failure) of SSR. This could lead to possible memory leaks in custom SSR apps, if their singleton services' `ngOnDestroy` hooks contained an important teardown logic (e.g. unsubscribing from RxJS observable). Note: I needed to fix by the way another thing too: now we destroy `moduleRef` when `platformInjector` is destroyed - by setting a `PLATFORM_DESTROY_LISTENER` fixes #58111
1 parent 5c63fc4 commit 0d957c2

File tree

4 files changed

+198
-27
lines changed

4 files changed

+198
-27
lines changed

‎packages/core/src/platform/bootstrap.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,21 +26,24 @@ import {Injector} from '../di';
2626
import {InternalNgModuleRef, NgModuleRef} from '../linker/ng_module_factory';
2727
import {stringify} from '../util/stringify';
2828

29-
export interface ModuleBootstrapConfig<M> {
29+
export interface BootstrapConfig {
30+
platformInjector: Injector;
31+
}
32+
33+
export interface ModuleBootstrapConfig<M> extends BootstrapConfig {
3034
moduleRef: InternalNgModuleRef<M>;
3135
allPlatformModules: NgModuleRef<unknown>[];
3236
}
3337

34-
export interface ApplicationBootstrapConfig {
38+
export interface ApplicationBootstrapConfig extends BootstrapConfig {
3539
r3Injector: R3Injector;
36-
platformInjector: Injector;
3740
rootComponent: Type<unknown> | undefined;
3841
}
3942

4043
function isApplicationBootstrapConfig(
4144
config: ApplicationBootstrapConfig | ModuleBootstrapConfig<unknown>,
4245
): config is ApplicationBootstrapConfig {
43-
return !!(config as ApplicationBootstrapConfig).platformInjector;
46+
return !(config as ModuleBootstrapConfig<unknown>).moduleRef;
4447
}
4548

4649
export function bootstrap<M>(
@@ -91,9 +94,9 @@ export function bootstrap<M>(
9194
});
9295
});
9396

97+
// If the whole platform is destroyed, invoke the `destroy` method
98+
// for all bootstrapped applications as well.
9499
if (isApplicationBootstrapConfig(config)) {
95-
// If the whole platform is destroyed, invoke the `destroy` method
96-
// for all bootstrapped applications as well.
97100
const destroyListener = () => envInjector.destroy();
98101
const onPlatformDestroyListeners = config.platformInjector.get(PLATFORM_DESTROY_LISTENERS);
99102
onPlatformDestroyListeners.add(destroyListener);
@@ -103,9 +106,14 @@ export function bootstrap<M>(
103106
onPlatformDestroyListeners.delete(destroyListener);
104107
});
105108
} else {
109+
const destroyListener = () => config.moduleRef.destroy();
110+
const onPlatformDestroyListeners = config.platformInjector.get(PLATFORM_DESTROY_LISTENERS);
111+
onPlatformDestroyListeners.add(destroyListener);
112+
106113
config.moduleRef.onDestroy(() => {
107114
remove(config.allPlatformModules, config.moduleRef);
108115
onErrorSubscription.unsubscribe();
116+
onPlatformDestroyListeners.delete(destroyListener);
109117
});
110118
}
111119

‎packages/core/src/platform/platform_ref.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,11 @@ export class PlatformRef {
7979
allAppProviders,
8080
);
8181

82-
return bootstrap({moduleRef, allPlatformModules: this._modules});
82+
return bootstrap({
83+
moduleRef,
84+
allPlatformModules: this._modules,
85+
platformInjector: this.injector,
86+
});
8387
}
8488

8589
/**

‎packages/platform-server/src/utils.ts‎

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -210,18 +210,21 @@ async function _render(platformRef: PlatformRef, applicationRef: ApplicationRef)
210210
}
211211

212212
appendServerContextInfo(applicationRef);
213-
const output = platformState.renderToString();
214213

215-
// Destroy the application in a macrotask, this allows pending promises to be settled and errors
216-
// to be surfaced to the users.
217-
await new Promise<void>((resolve) => {
214+
return platformState.renderToString();
215+
}
216+
217+
/**
218+
* Destroy the application in a macrotask, this allows pending promises to be settled and errors
219+
* to be surfaced to the users.
220+
*/
221+
function asyncDestroyPlatform(platformRef: PlatformRef): Promise<void> {
222+
return new Promise<void>((resolve) => {
218223
setTimeout(() => {
219224
platformRef.destroy();
220225
resolve();
221226
}, 0);
222227
});
223-
224-
return output;
225228
}
226229

227230
/**
@@ -264,9 +267,13 @@ export async function renderModule<T>(
264267
): Promise<string> {
265268
const {document, url, extraProviders: platformProviders} = options;
266269
const platformRef = createServerPlatform({document, url, platformProviders});
267-
const moduleRef = await platformRef.bootstrapModule(moduleType);
268-
const applicationRef = moduleRef.injector.get(ApplicationRef);
269-
return _render(platformRef, applicationRef);
270+
try {
271+
const moduleRef = await platformRef.bootstrapModule(moduleType);
272+
const applicationRef = moduleRef.injector.get(ApplicationRef);
273+
return await _render(platformRef, applicationRef);
274+
} finally {
275+
await asyncDestroyPlatform(platformRef);
276+
}
270277
}
271278

272279
/**
@@ -299,15 +306,17 @@ export async function renderApplication<T>(
299306

300307
startMeasuring(renderAppLabel);
301308
const platformRef = createServerPlatform(options);
302-
303-
startMeasuring(bootstrapLabel);
304-
const applicationRef = await bootstrap();
305-
stopMeasuring(bootstrapLabel);
306-
307-
startMeasuring(_renderLabel);
308-
const rendered = await _render(platformRef, applicationRef);
309-
stopMeasuring(_renderLabel);
310-
311-
stopMeasuring(renderAppLabel);
312-
return rendered;
309+
try {
310+
startMeasuring(bootstrapLabel);
311+
const applicationRef = await bootstrap();
312+
stopMeasuring(bootstrapLabel);
313+
314+
startMeasuring(_renderLabel);
315+
const rendered = await _render(platformRef, applicationRef);
316+
stopMeasuring(_renderLabel);
317+
return rendered;
318+
} finally {
319+
await asyncDestroyPlatform(platformRef);
320+
stopMeasuring(renderAppLabel);
321+
}
313322
}

‎packages/platform-server/test/integration_spec.ts‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ import {
4242
ViewEncapsulation,
4343
ɵPendingTasks as PendingTasks,
4444
ɵwhenStable as whenStable,
45+
APP_INITIALIZER,
46+
inject,
47+
getPlatform,
4548
} from '@angular/core';
4649
import {SSR_CONTENT_INTEGRITY_MARKER} from '@angular/core/src/hydration/utils';
4750
import {TestBed} from '@angular/core/testing';
@@ -1076,6 +1079,153 @@ class HiddenModule {}
10761079
);
10771080
},
10781081
);
1082+
1083+
it(
1084+
`should call onOnDestroy of a service after a successful render` +
1085+
`(standalone: ${isStandalone})`,
1086+
async () => {
1087+
let wasServiceNgOnDestroyCalled = false;
1088+
1089+
@Injectable({providedIn: 'root'})
1090+
class DestroyableService {
1091+
ngOnDestroy() {
1092+
wasServiceNgOnDestroyCalled = true;
1093+
}
1094+
}
1095+
1096+
const SuccessfulAppInitializerProviders = [
1097+
{
1098+
provide: APP_INITIALIZER,
1099+
useFactory: () => {
1100+
inject(DestroyableService);
1101+
return () => Promise.resolve(); // Success in APP_INITIALIZER
1102+
},
1103+
multi: true,
1104+
},
1105+
];
1106+
1107+
@NgModule({
1108+
providers: SuccessfulAppInitializerProviders,
1109+
imports: [MyServerAppModule, ServerModule],
1110+
bootstrap: [MyServerApp],
1111+
})
1112+
class ServerSuccessfulAppInitializerModule {}
1113+
1114+
const ServerSuccessfulAppInitializerAppStandalone = getStandaloneBootstrapFn(
1115+
createMyServerApp(true),
1116+
SuccessfulAppInitializerProviders,
1117+
);
1118+
1119+
const options = {document: doc};
1120+
const bootstrap = isStandalone
1121+
? renderApplication(ServerSuccessfulAppInitializerAppStandalone, options)
1122+
: renderModule(ServerSuccessfulAppInitializerModule, options);
1123+
await bootstrap;
1124+
1125+
expect(getPlatform()).withContext('PlatformRef should be destroyed').toBeNull();
1126+
expect(wasServiceNgOnDestroyCalled)
1127+
.withContext('DestroyableService.ngOnDestroy() should be called')
1128+
.toBeTrue();
1129+
},
1130+
);
1131+
1132+
it(
1133+
`should call onOnDestroy of a service after some APP_INITIALIZER fails ` +
1134+
`(standalone: ${isStandalone})`,
1135+
async () => {
1136+
let wasServiceNgOnDestroyCalled = false;
1137+
1138+
@Injectable({providedIn: 'root'})
1139+
class DestroyableService {
1140+
ngOnDestroy() {
1141+
wasServiceNgOnDestroyCalled = true;
1142+
}
1143+
}
1144+
1145+
const FailingAppInitializerProviders = [
1146+
{
1147+
provide: APP_INITIALIZER,
1148+
useFactory: () => {
1149+
inject(DestroyableService);
1150+
return () => Promise.reject('Error in APP_INITIALIZER');
1151+
},
1152+
multi: true,
1153+
},
1154+
];
1155+
1156+
@NgModule({
1157+
providers: FailingAppInitializerProviders,
1158+
imports: [MyServerAppModule, ServerModule],
1159+
bootstrap: [MyServerApp],
1160+
})
1161+
class ServerFailingAppInitializerModule {}
1162+
1163+
const ServerFailingAppInitializerAppStandalone = getStandaloneBootstrapFn(
1164+
createMyServerApp(true),
1165+
FailingAppInitializerProviders,
1166+
);
1167+
1168+
const options = {document: doc};
1169+
const bootstrap = isStandalone
1170+
? renderApplication(ServerFailingAppInitializerAppStandalone, options)
1171+
: renderModule(ServerFailingAppInitializerModule, options);
1172+
await expectAsync(bootstrap).toBeRejectedWith('Error in APP_INITIALIZER');
1173+
1174+
expect(getPlatform()).withContext('PlatformRef should be destroyed').toBeNull();
1175+
expect(wasServiceNgOnDestroyCalled)
1176+
.withContext('DestroyableService.ngOnDestroy() should be called')
1177+
.toBeTrue();
1178+
},
1179+
);
1180+
1181+
it(
1182+
`should call onOnDestroy of a service after an error happens in a root component's constructor ` +
1183+
`(standalone: ${isStandalone})`,
1184+
async () => {
1185+
let wasServiceNgOnDestroyCalled = false;
1186+
1187+
@Injectable({providedIn: 'root'})
1188+
class DestroyableService {
1189+
ngOnDestroy() {
1190+
wasServiceNgOnDestroyCalled = true;
1191+
}
1192+
}
1193+
1194+
@Component({
1195+
standalone: isStandalone,
1196+
selector: 'app',
1197+
template: `Works!`,
1198+
})
1199+
class MyServerFailingConstructorApp {
1200+
constructor() {
1201+
inject(DestroyableService);
1202+
throw 'Error in constructor of the root component';
1203+
}
1204+
}
1205+
1206+
@NgModule({
1207+
declarations: [MyServerFailingConstructorApp],
1208+
imports: [MyServerAppModule, ServerModule],
1209+
bootstrap: [MyServerFailingConstructorApp],
1210+
})
1211+
class MyServerFailingConstructorAppModule {}
1212+
1213+
const MyServerFailingConstructorAppStandalone = getStandaloneBootstrapFn(
1214+
MyServerFailingConstructorApp,
1215+
);
1216+
const options = {document: doc};
1217+
const bootstrap = isStandalone
1218+
? renderApplication(MyServerFailingConstructorAppStandalone, options)
1219+
: renderModule(MyServerFailingConstructorAppModule, options);
1220+
await expectAsync(bootstrap).toBeRejectedWith(
1221+
'Error in constructor of the root component',
1222+
);
1223+
expect(getPlatform()).withContext('PlatformRef should be destroyed').toBeNull();
1224+
expect(wasServiceNgOnDestroyCalled)
1225+
.withContext('DestroyableService.ngOnDestroy() should be called')
1226+
.toBeTrue();
1227+
},
1228+
);
10791229
});
10801230
});
10811231

0 commit comments

Comments
 (0)