Merge pull request #10696 from kos984/fix/NestApplicationContext-list…

…enToShutdownSignals fix(core): process exit before shutdown hook end
nestjs · Feb 1, 2023 · f71554c · f71554c
2 parents a986e7e + ceea86d
commit f71554c
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 1 deletion.
diff --git a/packages/core/nest-application-context.ts b/packages/core/nest-application-context.ts
@@ -323,13 +323,20 @@ export class NestApplicationContext<
    * @param {string[]} signals The system signals it should listen to
    */
   protected listenToShutdownSignals(signals: string[]) {
+    let receivedSignal = false;
     const cleanup = async (signal: string) => {
       try {
-        signals.forEach(sig => process.removeListener(sig, cleanup));
+        if (receivedSignal) {
+          // If we receive another signal while we're waiting
+          // for the server to stop, just ignore it.
+          return;
+        }
+        receivedSignal = true;
         await this.callDestroyHook();
         await this.callBeforeShutdownHook(signal);
         await this.dispose();
         await this.callShutdownHook(signal);
+        signals.forEach(sig => process.removeListener(sig, cleanup));
         process.kill(process.pid, signal);
       } catch (err) {
         Logger.error(

diff --git a/packages/core/test/nest-application-context.spec.ts b/packages/core/test/nest-application-context.spec.ts
@@ -6,6 +6,7 @@ import { Injector } from '../injector/injector';
 import { InstanceLoader } from '../injector/instance-loader';
 import { GraphInspector } from '../inspector/graph-inspector';
 import { NestApplicationContext } from '../nest-application-context';
+import * as sinon from 'sinon';
 
 describe('NestApplicationContext', () => {
   class A {}
@@ -49,6 +50,54 @@ describe('NestApplicationContext', () => {
     return applicationContext;
   }
 
+  describe('listenToShutdownSignals', () => {
+    it('shutdown process should not be interrupted by another handler', async () => {
+      const signal = 'SIGTERM';
+      let processUp = true;
+      let promisesResolved = false;
+      const applicationContext = await testHelper(A, Scope.DEFAULT);
+      applicationContext.enableShutdownHooks([signal]);
+
+      const waitProcessDown = new Promise(resolve => {
+        const shutdownCleanupRef = applicationContext['shutdownCleanupRef'];
+        const handler = () => {
+          if (
+            !process
+              .listeners(signal)
+              .find(handler => handler == shutdownCleanupRef)
+          ) {
+            processUp = false;
+            process.removeListener(signal, handler);
+            resolve(undefined);
+          }
+          return undefined;
+        };
+        process.on(signal, handler);
+      });
+
+      // add some third party handler
+      process.on(signal, signal => {
+        // do some work
+        process.kill(process.pid, signal);
+      });
+
+      const hookStub = sinon
+        .stub(applicationContext as any, 'callShutdownHook')
+        .callsFake(async () => {
+          // run some async code
+          await new Promise(resolve => setImmediate(() => resolve(undefined)));
+          if (processUp) {
+            promisesResolved = true;
+          }
+        });
+      process.kill(process.pid, signal);
+      await waitProcessDown;
+      hookStub.restore();
+      expect(processUp).to.be.false;
+      expect(promisesResolved).to.be.true;
+    });
+  });
+
   describe('get', () => {
     describe('when scope = DEFAULT', () => {
       it('should get value with function injection key', async () => {