Skip to content

Commit

Permalink
fix(extension-health): unregister signal listener to avoid potential …
Browse files Browse the repository at this point in the history
…memory leak

(node:93642) MaxListenersExceededWarning: Possible EventEmitter memory
leak detected. 11 SIGTERM listeners added to [process]. Use emitter.setMaxListeners()
to increase limit

Signed-off-by: Raymond Feng <enjoyjava@gmail.com>
  • Loading branch information
raymondfeng committed Aug 28, 2020
1 parent a3f5427 commit 731c987
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 5 deletions.
12 changes: 12 additions & 0 deletions extensions/health/src/__tests__/acceptance/health.acceptance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {RestApplication, RestServerConfig} from '@loopback/rest';
import {
Client,
createRestAppClient,
expect,
givenHttpServerConfig,
} from '@loopback/testlab';
import {HealthComponent} from '../..';
Expand Down Expand Up @@ -41,6 +42,17 @@ describe('Health (acceptance)', () => {
it('exposes health at "/live/"', async () => {
await request.get('/live').expect(200, {status: 'UP', checks: []});
});

it('removes listener from the process', async () => {
const healthChecker = await app.get(HealthBindings.HEALTH_CHECKER);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const onShutdownRequest = (healthChecker as any).onShutdownRequest;
let listeners = process.listeners('SIGTERM');
expect(listeners).to.containEql(onShutdownRequest);
await app.stop();
listeners = process.listeners('SIGTERM');
expect(listeners).to.not.containEql(onShutdownRequest);
});
});

context('with discovered live/ready checks', () => {
Expand Down
17 changes: 12 additions & 5 deletions extensions/health/src/observers/health.observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {LiveCheck, ReadyCheck} from '../types';
export class HealthObserver implements LifeCycleObserver {
private eventEmitter = new EventEmitter();
private startupCheck: Promise<void>;
private shutdownCheck: ShutdownCheck;

constructor(
@inject(HealthBindings.HEALTH_CHECKER) private healthChecker: HealthChecker,
Expand All @@ -37,15 +38,13 @@ export class HealthObserver implements LifeCycleObserver {
void
>;
const startupCheck = new StartupCheck('startup', () => startup);

const shutdown = once(this.eventEmitter, 'shutdown');
const shutdownCheck = new ShutdownCheck('shutdown', () => shutdown);

this.startupCheck = this.healthChecker.registerStartupCheck(startupCheck);
this.healthChecker.registerShutdownCheck(shutdownCheck);
const shutdown = once(this.eventEmitter, 'shutdown');
this.shutdownCheck = new ShutdownCheck('shutdown', () => shutdown);
}

async start() {
this.healthChecker.registerShutdownCheck(this.shutdownCheck);
const liveChecks = await this.liveChecks.values();
const liveCheckBindings = this.liveChecks.bindings;
let index = 0;
Expand All @@ -72,6 +71,14 @@ export class HealthObserver implements LifeCycleObserver {

stop() {
this.eventEmitter.emit('shutdown');
// Fix a potential memory leak caused by
// https://github.com/CloudNativeJS/cloud-health/blob/2.1.2/src/healthcheck/HealthChecker.ts#L118
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const onShutdownRequest = (this.healthChecker as any).onShutdownRequest;
if (onShutdownRequest != null) {
// Remove the listener from the current process
process.removeListener('SIGTERM', onShutdownRequest);
}
}
}

Expand Down

0 comments on commit 731c987

Please sign in to comment.