Skip to content

Commit 83ec279

Browse files
BrunnerLivioBrunnerLivio
committed
fix(@nestjs/core): Fix process.on memory leak bug in e2e testing
1 parent 0cc1c4e commit 83ec279

8 files changed

Lines changed: 146 additions & 24 deletions

File tree

‎integration/hooks/e2e/on-app-shutdown.spec.ts‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { Test } from '@nestjs/testing';
22
import { expect } from 'chai';
33
import * as Sinon from 'sinon';
4-
import { Injectable, OnApplicationShutdown } from '@nestjs/common';
5-
import { spawn } from 'child_process';
4+
import { Injectable, OnApplicationShutdown, ShutdownSignal } from '@nestjs/common';
5+
import { spawn, spawnSync } from 'child_process';
6+
import { join } from 'path';
67

78
@Injectable()
89
class TestInjectable implements OnApplicationShutdown {
@@ -20,4 +21,28 @@ describe('OnApplicationShutdown', () => {
2021
const instance = module.get(TestInjectable);
2122
expect(instance.onApplicationShutdown.called).to.be.true;
2223
});
24+
25+
it('should call onApplicationShutdown if any shutdown signal gets invoked', done => {
26+
const result = spawnSync('ts-node', [join(__dirname, '../src/main.ts'), 'SIGHUP']);
27+
expect(result.stdout.toString().trim()).to.be.eq('Signal SIGHUP');
28+
done();
29+
});
30+
31+
it('should call onApplicationShutdown if a specific shutdown signal gets invoked', done => {
32+
const result = spawnSync('ts-node', [join(__dirname, '../src/main.ts'), 'SIGINT', 'SIGINT']);
33+
expect(result.stdout.toString().trim()).to.be.eq('Signal SIGINT');
34+
done();
35+
});
36+
37+
it('should ignore system signals which are not specified', done => {
38+
const result = spawnSync('ts-node', [join(__dirname, '../src/main.ts'), 'SIGINT', 'SIGHUP']);
39+
expect(result.stdout.toString().trim()).to.be.eq('');
40+
done();
41+
});
42+
43+
it('should ignore system signals if "enableShutdownHooks" was not called', done => {
44+
const result = spawnSync('ts-node', [join(__dirname, '../src/main.ts'), 'SIGINT', 'NONE']);
45+
expect(result.stdout.toString().trim()).to.be.eq('');
46+
done();
47+
});
2348
});

‎integration/hooks/src/main.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import { Injectable, OnApplicationShutdown, Module } from '@nestjs/common';
2+
import { NestFactory } from '@nestjs/core';
3+
const SIGNAL = process.argv[2];
4+
const SIGNAL_TO_LISTEN = process.argv[3];
5+
6+
@Injectable()
7+
class TestInjectable implements OnApplicationShutdown {
8+
onApplicationShutdown(signal: string) {
9+
console.log('Signal ' + signal);
10+
}
11+
}
12+
13+
@Module({
14+
providers: [TestInjectable],
15+
})
16+
class AppModule { }
17+
18+
async function bootstrap() {
19+
const app = await NestFactory.create(AppModule, { logger: true });
20+
21+
if (SIGNAL_TO_LISTEN && SIGNAL_TO_LISTEN !== 'NONE') {
22+
app.enableShutdownHooks([SIGNAL_TO_LISTEN]);
23+
} else if (SIGNAL_TO_LISTEN !== 'NONE') {
24+
app.enableShutdownHooks();
25+
}
26+
27+
await app.listen(1800);
28+
process.kill(process.pid, SIGNAL);
29+
}
30+
31+
bootstrap();

‎packages/common/enums/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
export * from './request-method.enum';
22
export * from './http-status.enum';
3+
export * from './shutdown-signal.enum';

‎packages/common/enums/shutdown-signal.enum.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/**
2+
* System signals which shut down a process
3+
*/
4+
export enum ShutdownSignal {
5+
SIGHUP = 'SIGHUP',
6+
SIGINT = 'SIGINT',
7+
SIGQUIT = 'SIGQUIT',
8+
SIGILL = 'SIGILL',
9+
SIGTRAP = 'SIGTRAP',
10+
SIGABRT = 'SIGABRT',
11+
SIGBUS = 'SIGBUS',
12+
SIGFPE = 'SIGFPE',
13+
SIGSEGV = 'SIGSEGV',
14+
SIGUSR2 = 'SIGUSR2',
15+
SIGTERM = 'SIGTERM',
16+
}

‎packages/common/interfaces/nest-application-context.interface.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { LoggerService } from '../services/logger.service';
22
import { Abstract } from './abstract.interface';
33
import { Type } from './type.interface';
4+
import { ShutdownSignal } from '../enums/shutdown-signal.enum';
45

56
export interface INestApplicationContext {
67
/**
@@ -29,4 +30,13 @@ export interface INestApplicationContext {
2930
* @returns {void}
3031
*/
3132
useLogger(logger: LoggerService): void;
33+
34+
/**
35+
* Enables the usage of shutdown hooks. Will call the
36+
* `onApplicationShutdown` function of a provider if the
37+
* process receives a shutdown signal.
38+
*
39+
* @returns {this} The Nest application context instance
40+
*/
41+
enableShutdownHooks(signals?: ShutdownSignal[] | string[]): this;
3242
}

‎packages/core/constants.ts‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,3 @@ export const APP_INTERCEPTOR = 'APP_INTERCEPTOR';
99
export const APP_PIPE = 'APP_PIPE';
1010
export const APP_GUARD = 'APP_GUARD';
1111
export const APP_FILTER = 'APP_FILTER';
12-
export const SHUTDOWN_SIGNALS = [
13-
'SIGHUP',
14-
'SIGINT',
15-
'SIGQUIT',
16-
'SIGILL',
17-
'SIGTRAP',
18-
'SIGABRT',
19-
'SIGBUS',
20-
'SIGFPE',
21-
'SIGSEGV',
22-
'SIGUSR2',
23-
'SIGTERM',
24-
];

‎packages/core/nest-application-context.ts‎

Lines changed: 61 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
1-
import { INestApplicationContext, Logger, LoggerService } from '@nestjs/common';
1+
import {
2+
INestApplicationContext,
3+
Logger,
4+
LoggerService,
5+
ShutdownSignal,
6+
} from '@nestjs/common';
27
import { Abstract } from '@nestjs/common/interfaces';
38
import { Type } from '@nestjs/common/interfaces/type.interface';
4-
import { SHUTDOWN_SIGNALS } from './constants';
59
import { UnknownModuleException } from './errors/exceptions/unknown-module.exception';
610
import {
711
callAppShutdownHook,
@@ -17,6 +21,7 @@ import { ModuleTokenFactory } from './injector/module-token-factory';
1721
export class NestApplicationContext implements INestApplicationContext {
1822
private readonly moduleTokenFactory = new ModuleTokenFactory();
1923
private readonly containerScanner: ContainerScanner;
24+
private readonly activeShutdownSignals: string[] = new Array<string>();
2025

2126
constructor(
2227
protected readonly container: NestContainer,
@@ -60,7 +65,6 @@ export class NestApplicationContext implements INestApplicationContext {
6065
public async init(): Promise<this> {
6166
await this.callInitHook();
6267
await this.callBootstrapHook();
63-
await this.listenToShutdownSignals();
6468
return this;
6569
}
6670

@@ -73,14 +77,63 @@ export class NestApplicationContext implements INestApplicationContext {
7377
Logger.overrideLogger(logger);
7478
}
7579

76-
protected listenToShutdownSignals() {
77-
SHUTDOWN_SIGNALS.forEach((signal: any) =>
78-
process.on(signal, async code => {
80+
/**
81+
* Enables the usage of shutdown hooks. Will call the
82+
* `onApplicationShutdown` function of a provider if the
83+
* process receives a shutdown signal.
84+
*
85+
* @param {ShutdownSignal[]} [signals=[]] The system signals it should listen to
86+
*
87+
* @returns {this} The Nest application context instance
88+
*/
89+
public enableShutdownHooks(signals: (ShutdownSignal | string)[] = []): this {
90+
if (signals.length === 0) {
91+
signals = Object.keys(ShutdownSignal).map(
92+
(key: string) => ShutdownSignal[key],
93+
);
94+
} else {
95+
// Given signals array should be unique because
96+
// process shouldn't listen to the same signal more than once.
97+
signals = Array.from(new Set(signals));
98+
}
99+
100+
signals = signals
101+
// To string and consistentify
102+
.map(
103+
(signal: ShutdownSignal | string): string =>
104+
signal
105+
.toString()
106+
.toUpperCase()
107+
.trim(),
108+
)
109+
// Filter out the signals which is already listening to
110+
.filter(
111+
(signal: string) => !this.activeShutdownSignals.includes(signal),
112+
) as string[];
113+
114+
this.listenToShutdownSignals(signals);
115+
return this;
116+
}
117+
118+
/**
119+
* Listens to shutdown signals by listening to
120+
* process events
121+
*
122+
* @param {string[]} signals The system signals it should listen to
123+
*/
124+
protected listenToShutdownSignals(signals: string[]) {
125+
signals.forEach((signal: string) => {
126+
this.activeShutdownSignals.push(signal);
127+
128+
process.on(signal as any, async code => {
129+
// Call the destroy and shutdown hook
130+
// in case the process receives a shutdown signal
79131
await this.callDestroyHook();
80132
await this.callShutdownHook(signal);
133+
81134
process.exit(code || 1);
82-
}),
83-
);
135+
});
136+
});
84137
}
85138

86139
/**

‎packages/core/nest-application.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,6 @@ export class NestApplication extends NestApplicationContext
126126
await this.callInitHook();
127127
await this.registerRouterHooks();
128128
await this.callBootstrapHook();
129-
await this.listenToShutdownSignals();
130129

131130
this.isInitialized = true;
132131
this.logger.log(MESSAGES.APPLICATION_READY);

0 commit comments

Comments
 (0)