From 0815b9401d087202cd64458b6906a5225929fc5d Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 8 May 2016 03:28:47 +0200 Subject: vm: add ability to break on sigint/ctrl+c - Adds the `breakEvalOnSigint` option to `vm.runIn(This)Context`. This uses a watchdog thread to wait for SIGINT and generally works just like the existing `timeout` option. - Adds a method to the existing timer-based watchdog to check if it stopped regularly or by running into the timeout. This is used to tell a SIGINT abort from a timer-based one. - Adds (internal) `process._{start,stop}SigintWatchdog` methods to start/stop the watchdog thread used by the above option manually. This will be used in the REPL to set up SIGINT handling before entering terminal raw mode, so that there is no time window in which Ctrl+C fully aborts the process. PR-URL: https://github.com/nodejs/node/pull/6635 Reviewed-By: Ben Noordhuis --- doc/api/vm.md | 6 + lib/vm.js | 47 +++++ src/node.cc | 8 +- src/node_contextify.cc | 42 +++- src/node_internals.h | 7 + src/node_util.cc | 18 ++ src/node_watchdog.cc | 225 +++++++++++++++++++++ src/node_watchdog.h | 59 +++++- test/message/eval_messages.out | 3 + test/message/stdin_messages.out | 3 + .../message/undefined_reference_in_new_context.out | 2 +- test/message/vm_display_runtime_error.out | 2 +- test/message/vm_dont_display_runtime_error.out | 2 +- test/parallel/test-util-sigint-watchdog.js | 53 +++++ test/parallel/test-vm-sigint-existing-handler.js | 77 +++++++ test/parallel/test-vm-sigint.js | 39 ++++ 16 files changed, 582 insertions(+), 11 deletions(-) create mode 100644 test/parallel/test-util-sigint-watchdog.js create mode 100644 test/parallel/test-vm-sigint-existing-handler.js create mode 100644 test/parallel/test-vm-sigint.js diff --git a/doc/api/vm.md b/doc/api/vm.md index b65826b2d3..0141ad605e 100644 --- a/doc/api/vm.md +++ b/doc/api/vm.md @@ -77,6 +77,12 @@ added: v0.3.1 * `timeout` {number} Specifies the number of milliseconds to execute `code` before terminating execution. If execution is terminated, an [`Error`][] will be thrown. + * `breakOnSigint`: if `true`, the execution will be terminated when + `SIGINT` (Ctrl+C) is received. Existing handlers for the + event that have been attached via `process.on("SIGINT")` will be disabled + during script execution, but will continue to work after that. + If execution is terminated, an [`Error`][] will be thrown. + Runs the compiled code contained by the `vm.Script` object within the given `contextifiedSandbox` and returns the result. Running code does not have access diff --git a/lib/vm.js b/lib/vm.js index b4a2b99990..364a37eacb 100644 --- a/lib/vm.js +++ b/lib/vm.js @@ -13,6 +13,29 @@ const Script = binding.ContextifyScript; // - isContext(sandbox) // From this we build the entire documented API. +const realRunInThisContext = Script.prototype.runInThisContext; +const realRunInContext = Script.prototype.runInContext; + +Script.prototype.runInThisContext = function(options) { + if (options && options.breakOnSigint) { + return sigintHandlersWrap(() => { + return realRunInThisContext.call(this, options); + }); + } else { + return realRunInThisContext.call(this, options); + } +}; + +Script.prototype.runInContext = function(contextifiedSandbox, options) { + if (options && options.breakOnSigint) { + return sigintHandlersWrap(() => { + return realRunInContext.call(this, contextifiedSandbox, options); + }); + } else { + return realRunInContext.call(this, contextifiedSandbox, options); + } +}; + Script.prototype.runInNewContext = function(sandbox, options) { var context = exports.createContext(sandbox); return this.runInContext(context, options); @@ -55,3 +78,27 @@ exports.runInThisContext = function(code, options) { }; exports.isContext = binding.isContext; + +// 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) { + // Using the internal list here to make sure `.once()` wrappers are used, + // not the original ones. + let sigintListeners = process._events.SIGINT; + if (!Array.isArray(sigintListeners)) + sigintListeners = sigintListeners ? [sigintListeners] : []; + else + sigintListeners = sigintListeners.slice(); + + process.removeAllListeners('SIGINT'); + + try { + return fn(); + } finally { + // Add using the public methods so that the `newListener` handler of + // process can re-attach the listeners. + for (const listener of sigintListeners) { + process.addListener('SIGINT', listener); + } + } +} diff --git a/src/node.cc b/src/node.cc index e511f900a7..3886be6de6 100644 --- a/src/node.cc +++ b/src/node.cc @@ -3239,7 +3239,7 @@ static void AtExit() { } -static void SignalExit(int signo) { +void SignalExit(int signo) { uv_tty_reset_mode(); #ifdef __FreeBSD__ // FreeBSD has a nasty bug, see RegisterSignalHandler for details @@ -3735,9 +3735,9 @@ static void EnableDebugSignalHandler(int signo) { } -static void RegisterSignalHandler(int signal, - void (*handler)(int signal), - bool reset_handler = false) { +void RegisterSignalHandler(int signal, + void (*handler)(int signal), + bool reset_handler) { struct sigaction sa; memset(&sa, 0, sizeof(sa)); sa.sa_handler = handler; diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 774871b852..a4a7693594 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -553,6 +553,7 @@ class ContextifyScript : public BaseObject { TryCatch try_catch(args.GetIsolate()); uint64_t timeout = GetTimeoutArg(args, 0); bool display_errors = GetDisplayErrorsArg(args, 0); + bool break_on_sigint = GetBreakOnSigintArg(args, 0); if (try_catch.HasCaught()) { try_catch.ReThrow(); return; @@ -560,7 +561,7 @@ class ContextifyScript : public BaseObject { // Do the eval within this context Environment* env = Environment::GetCurrent(args); - EvalMachine(env, timeout, display_errors, args, try_catch); + EvalMachine(env, timeout, display_errors, break_on_sigint, args, try_catch); } // args: sandbox, [options] @@ -569,6 +570,7 @@ class ContextifyScript : public BaseObject { int64_t timeout; bool display_errors; + bool break_on_sigint; // Assemble arguments if (!args[0]->IsObject()) { @@ -581,6 +583,7 @@ class ContextifyScript : public BaseObject { TryCatch try_catch(env->isolate()); timeout = GetTimeoutArg(args, 1); display_errors = GetDisplayErrorsArg(args, 1); + break_on_sigint = GetBreakOnSigintArg(args, 1); if (try_catch.HasCaught()) { try_catch.ReThrow(); return; @@ -605,6 +608,7 @@ class ContextifyScript : public BaseObject { if (EvalMachine(contextify_context->env(), timeout, display_errors, + break_on_sigint, args, try_catch)) { contextify_context->CopyProperties(); @@ -653,6 +657,23 @@ class ContextifyScript : public BaseObject { True(env->isolate())); } + static bool GetBreakOnSigintArg(const FunctionCallbackInfo& args, + const int i) { + if (args[i]->IsUndefined() || args[i]->IsString()) { + return false; + } + if (!args[i]->IsObject()) { + Environment::ThrowTypeError(args.GetIsolate(), + "options must be an object"); + return false; + } + + Local key = FIXED_ONE_BYTE_STRING(args.GetIsolate(), + "breakOnSigint"); + Local value = args[i].As()->Get(key); + return value->IsTrue(); + } + static int64_t GetTimeoutArg(const FunctionCallbackInfo& args, const int i) { if (args[i]->IsUndefined() || args[i]->IsString()) { @@ -798,6 +819,7 @@ class ContextifyScript : public BaseObject { static bool EvalMachine(Environment* env, const int64_t timeout, const bool display_errors, + const bool break_on_sigint, const FunctionCallbackInfo& args, TryCatch& try_catch) { if (!ContextifyScript::InstanceOf(env, args.Holder())) { @@ -813,16 +835,30 @@ class ContextifyScript : public BaseObject { Local