diff --git a/lib/internal/vm.js b/lib/internal/vm.js index 9f1c33b0ccfe..e6c5f4aaed69 100644 --- a/lib/internal/vm.js +++ b/lib/internal/vm.js @@ -1,7 +1,9 @@ 'use strict'; const { + ArrayPrototypeForEach, FunctionPrototypeCall, + ReflectApply, Symbol, } = primordials; @@ -111,6 +113,34 @@ function registerImportModuleDynamically(referrer, importModuleDynamically) { }); } +/** + * Temporarily removes all SIGINT listeners before invoking `fn`, re-attaching + * them afterwards. Used to run code with `breakOnSigint: true`, where a + * native SigintWatchdog is installed to terminate execution on SIGINT: if the + * process's own SIGINT listeners stayed attached, they would race with that + * watchdog over the same signal instead of running normally once the + * watchdog is gone. + * @param {Function} fn - The function to invoke with SIGINT listeners removed. + * @param {object} thisArg - The `this` value to invoke `fn` with. + * @param {Array} argsArray - The arguments to invoke `fn` with. + * @returns {any} + */ +function sigintHandlersWrap(fn, thisArg, argsArray) { + const sigintListeners = process.rawListeners('SIGINT'); + + process.removeAllListeners('SIGINT'); + + try { + return ReflectApply(fn, thisArg, argsArray); + } finally { + // Add using the public methods so that the `newListener` handler of + // process can re-attach the listeners. + ArrayPrototypeForEach(sigintListeners, (listener) => { + process.addListener('SIGINT', listener); + }); + } +} + /** * Compiles a function from the given code string. * @param {string} code - The code string to compile. @@ -234,4 +264,5 @@ module.exports = { makeContextifyScript, registerImportModuleDynamically, runScriptInThisContext, + sigintHandlersWrap, }; diff --git a/lib/internal/vm/module.js b/lib/internal/vm/module.js index ce2350f6cd16..464aa4d28031 100644 --- a/lib/internal/vm/module.js +++ b/lib/internal/vm/module.js @@ -92,7 +92,7 @@ const kContext = Symbol('kContext'); const kPerContextModuleId = Symbol('kPerContextModuleId'); const kLink = Symbol('kLink'); -const { isContext } = require('internal/vm'); +const { isContext, sigintHandlersWrap } = require('internal/vm'); function isModule(object) { if (typeof object !== 'object' || object === null || !ObjectPrototypeHasOwnProperty(object, kWrap)) { @@ -230,6 +230,10 @@ class Module { 'must be one of linked, evaluated, or errored', ); } + if (breakOnSigint && process.listenerCount('SIGINT') > 0) { + return sigintHandlersWrap( + this[kWrap].evaluate, this[kWrap], [timeout, breakOnSigint]); + } return this[kWrap].evaluate(timeout, breakOnSigint); } catch (e) { return PromiseReject(e); diff --git a/lib/vm.js b/lib/vm.js index 2c7446af6076..913c85a679df 100644 --- a/lib/vm.js +++ b/lib/vm.js @@ -63,6 +63,7 @@ const { internalCompileFunction, isContext: _isContext, registerImportModuleDynamically, + sigintHandlersWrap, } = require('internal/vm'); const { vm_dynamic_import_main_context_default, @@ -270,24 +271,6 @@ function createScript(code, options) { return new Script(code, options); } -// Remove all SIGINT listeners and re-attach them after the wrapped function -// has executed, so that caught SIGINT are handled by the listeners again. -function sigintHandlersWrap(fn, thisArg, argsArray) { - const sigintListeners = process.rawListeners('SIGINT'); - - process.removeAllListeners('SIGINT'); - - try { - return ReflectApply(fn, thisArg, argsArray); - } finally { - // Add using the public methods so that the `newListener` handler of - // process can re-attach the listeners. - ArrayPrototypeForEach(sigintListeners, (listener) => { - process.addListener('SIGINT', listener); - }); - } -} - function runInContext(code, contextifiedObject, options) { validateContext(contextifiedObject); if (typeof options === 'string') { diff --git a/test/parallel/test-vm-module-sigint-existing-handler.js b/test/parallel/test-vm-module-sigint-existing-handler.js new file mode 100644 index 000000000000..7d4a126cbdc0 --- /dev/null +++ b/test/parallel/test-vm-module-sigint-existing-handler.js @@ -0,0 +1,85 @@ +'use strict'; +const common = require('../common'); +if (common.isWindows) { + // No way to send CTRL_C_EVENT to processes from JS right now. + common.skip('platform not supported'); +} + +// Tests that vm.Module.prototype.evaluate({ breakOnSigint: true }) does not +// race with the process's own pre-existing SIGINT listeners, the same way +// vm.Script's runInThisContext()/runInContext() already don't (see +// test-vm-sigint-existing-handler.js). + +const assert = require('assert'); +const vm = require('vm'); +const spawn = require('child_process').spawn; + +if (process.argv[2] === 'child') { + let firstHandlerCalled = 0; + process.on('SIGINT', common.mustCall(() => { + firstHandlerCalled++; + // Handler attached _before_ execution. + }, 2)); + + let onceHandlerCalled = 0; + process.once('SIGINT', common.mustCall(() => { + onceHandlerCalled++; + // Handler attached _before_ execution. + })); + + (async () => { + const context = vm.createContext({ process }); + const mod = new vm.SourceTextModule( + 'process.send("ready"); while (true) {}', + { context }); + await mod.link(() => {}); + + await assert.rejects( + mod.evaluate({ breakOnSigint: true }), + { code: 'ERR_SCRIPT_EXECUTION_INTERRUPTED' }, + ); + assert.strictEqual(firstHandlerCalled, 0); + assert.strictEqual(onceHandlerCalled, 0); + + // Keep the process alive for a while so the second SIGINT can be caught. + const timeout = setTimeout(() => {}, 1000); + + let afterHandlerCalled = 0; + process.on('SIGINT', common.mustCall(() => { + // Handler attached _after_ execution. + if (afterHandlerCalled++ === 0) { + // The first time it just bounces back to check that the `once()` + // handler is not called the second time. + assert.strictEqual(firstHandlerCalled, 1); + assert.strictEqual(onceHandlerCalled, 1); + process.send('again'); + return; + } + + assert.strictEqual(onceHandlerCalled, 1); + assert.strictEqual(firstHandlerCalled, 2); + timeout.unref(); + }, 2)); + + process.send('again'); + })().then(common.mustCall()); + + return; +} + +const child = spawn(process.execPath, [ + '--experimental-vm-modules', __filename, 'child', +], { + stdio: [null, 'inherit', 'inherit', 'ipc'], +}); + +child.on('message', common.mustCall(() => { + // First kill() breaks the while(true) loop, second one invokes the real + // signal handlers. + process.kill(child.pid, 'SIGINT'); +}, 3)); + +child.on('close', common.mustCall((code, signal) => { + assert.strictEqual(signal, null); + assert.strictEqual(code, 0); +}));