From 6f1f6269643a274e235793262ab43414e8347323 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna.henningsen@mongodb.com>
Date: Wed, 17 Mar 2021 17:50:40 +0100
Subject: [PATCH] fix(cli-repl): allow interrupting load()ed code MONGOSH-641
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This was previously broken because the async repl implementation
adds an `process.on('SIGINT')` listener for the duration of the
*async* part of the evaluation. That listener would interfere with
breaking on sigint if another *synchronous* evaluation would be
started while it is present.

Fix this by not being lazy anymore and temporarily disabling
existing `SIGINT` event listeners on both `process` and `repl`
(and not just `repl`). In this case, the “inner” evaluation
also temporarily removes the “outer” `SIGINT` listener and
can go about as usual.
---
 packages/cli-repl/src/async-repl.ts           | 31 +++++++++++++++----
 packages/cli-repl/test/e2e.spec.ts            |  7 +++++
 .../test/fixtures/load/infinite-loop.js       |  2 ++
 3 files changed, 34 insertions(+), 6 deletions(-)
 create mode 100644 packages/cli-repl/test/fixtures/load/infinite-loop.js

diff --git a/packages/cli-repl/src/async-repl.ts b/packages/cli-repl/src/async-repl.ts
index c6100a40ac..175d9e86fb 100644
--- a/packages/cli-repl/src/async-repl.ts
+++ b/packages/cli-repl/src/async-repl.ts
@@ -1,5 +1,7 @@
+/* eslint-disable chai-friendly/no-unused-expressions */
 import type { REPLServer, ReplOptions } from 'repl';
 import type { ReadLineOptions } from 'readline';
+import type { EventEmitter } from 'events';
 import { Recoverable, start as originalStart } from 'repl';
 import isRecoverableError from 'is-recoverable-error';
 import { promisify } from 'util';
@@ -33,6 +35,20 @@ export type EvalFinishEvent = EvalStartEvent & ({
 export const evalStart = Symbol('async-repl:evalStart');
 export const evalFinish = Symbol('async-repl:evalFinish');
 
+// Helper for temporarily disabling an event on an EventEmitter.
+type RestoreEvents = { restore: () => void };
+function disableEvent(emitter: EventEmitter, event: string): RestoreEvents {
+  const rawListeners = emitter.rawListeners(event);
+  emitter.removeAllListeners(event);
+  return {
+    restore() {
+      for (const listener of rawListeners) {
+        emitter.on(event, listener as any);
+      }
+    }
+  };
+}
+
 // Start a REPLSever that supports asynchronous evaluation, rather than just
 // synchronous, and integrates nicely with Ctrl+C handling in that respect.
 export function start(opts: AsyncREPLOptions): REPLServer {
@@ -54,7 +70,8 @@ export function start(opts: AsyncREPLOptions): REPLServer {
       let previousExitListeners: any[] = [];
 
       let sigintListener: (() => void) | undefined = undefined;
-      let previousSigintListeners: any[] = [];
+      let replSigint: RestoreEvents | undefined = undefined;
+      let processSigint: RestoreEvents | undefined = undefined;
 
       try {
         result = await new Promise((resolve, reject) => {
@@ -67,9 +84,10 @@ export function start(opts: AsyncREPLOptions): REPLServer {
               // itself if the `customEval` itself is interrupted.
               reject(new Error('Asynchronous execution was interrupted by `SIGINT`'));
             };
-            previousSigintListeners = repl.rawListeners('SIGINT');
 
-            repl.removeAllListeners('SIGINT');
+            replSigint = disableEvent(repl, 'SIGINT');
+            processSigint = disableEvent(process, 'SIGINT');
+
             repl.once('SIGINT', sigintListener);
           }
 
@@ -93,9 +111,10 @@ export function start(opts: AsyncREPLOptions): REPLServer {
           repl.removeListener('SIGINT', sigintListener);
           process.removeListener('SIGINT', sigintListener);
         }
-        for (const listener of previousSigintListeners) {
-          repl.on('SIGINT', listener);
-        }
+        // Oh no, a TypeScript bug? 🐞 https://github.com/microsoft/TypeScript/issues/43287
+        // `as any` to the rescue!
+        (replSigint as any)?.restore?.();
+        (processSigint as any)?.restore?.();
 
         repl.removeListener('exit', exitListener);
         for (const listener of previousExitListeners) {
diff --git a/packages/cli-repl/test/e2e.spec.ts b/packages/cli-repl/test/e2e.spec.ts
index 49b9595c29..bc5e243f21 100644
--- a/packages/cli-repl/test/e2e.spec.ts
+++ b/packages/cli-repl/test/e2e.spec.ts
@@ -369,6 +369,13 @@ describe('e2e', function() {
       await result;
       shell.assertContainsError('interrupted');
     });
+    it('interrupts load()', async() => {
+      const filename = path.resolve(__dirname, 'fixtures', 'load', 'infinite-loop.js');
+      const result = shell.executeLine(`load(${JSON.stringify(filename)})`);
+      setTimeout(() => shell.kill('SIGINT'), 1000);
+      await result;
+      shell.assertContainsError('interrupted');
+    });
     it('behaves normally after an exception', async() => {
       await shell.executeLine('throw new Error()');
       await new Promise((resolve) => setTimeout(resolve, 100));
diff --git a/packages/cli-repl/test/fixtures/load/infinite-loop.js b/packages/cli-repl/test/fixtures/load/infinite-loop.js
new file mode 100644
index 0000000000..6029ab98f0
--- /dev/null
+++ b/packages/cli-repl/test/fixtures/load/infinite-loop.js
@@ -0,0 +1,2 @@
+// eslint-disable-next-line no-constant-condition
+while (true);