diff --git a/src/node_contextify.cc b/src/node_contextify.cc index a4a7693594..e5cbfd5512 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -836,14 +836,17 @@ class ContextifyScript : public BaseObject { Local result; bool timed_out = false; + bool received_signal = false; if (break_on_sigint && timeout != -1) { Watchdog wd(env->isolate(), timeout); SigintWatchdog swd(env->isolate()); result = script->Run(); timed_out = wd.HasTimedOut(); + received_signal = swd.HasReceivedSignal(); } else if (break_on_sigint) { SigintWatchdog swd(env->isolate()); result = script->Run(); + received_signal = swd.HasReceivedSignal(); } else if (timeout != -1) { Watchdog wd(env->isolate(), timeout); result = script->Run(); @@ -852,14 +855,26 @@ class ContextifyScript : public BaseObject { result = script->Run(); } - if (try_catch.HasCaught() && try_catch.HasTerminated()) { - env->isolate()->CancelTerminateExecution(); + if (try_catch.HasCaught()) { + if (try_catch.HasTerminated()) + env->isolate()->CancelTerminateExecution(); + + // It is possible that execution was terminated by another timeout in + // which this timeout is nested, so check whether one of the watchdogs + // from this invocation is responsible for termination. if (timed_out) { env->ThrowError("Script execution timed out."); - } else { + } else if (received_signal) { env->ThrowError("Script execution interrupted."); } + + // If there was an exception thrown during script execution, re-throw it. + // If one of the above checks threw, re-throw the exception instead of + // letting try_catch catch it. + // If execution has been terminated, but not by one of the watchdogs from + // this invocation, this will re-throw a `null` value. try_catch.ReThrow(); + return false; } diff --git a/src/node_watchdog.cc b/src/node_watchdog.cc index 8e94bc8d9d..8a067c27f3 100644 --- a/src/node_watchdog.cc +++ b/src/node_watchdog.cc @@ -99,7 +99,7 @@ void SigintWatchdog::Dispose() { SigintWatchdog::SigintWatchdog(v8::Isolate* isolate) - : isolate_(isolate), destroyed_(false) { + : isolate_(isolate), received_signal_(false), destroyed_(false) { // Register this watchdog with the global SIGINT/Ctrl+C listener. SigintWatchdogHelper::GetInstance()->Register(this); // Start the helper thread, if that has not already happened. @@ -120,6 +120,7 @@ void SigintWatchdog::Destroy() { void SigintWatchdog::HandleSigint() { + received_signal_ = true; isolate_->TerminateExecution(); } diff --git a/src/node_watchdog.h b/src/node_watchdog.h index 65815e2bcd..77c2d53534 100644 --- a/src/node_watchdog.h +++ b/src/node_watchdog.h @@ -46,11 +46,13 @@ class SigintWatchdog { void Dispose(); v8::Isolate* isolate() { return isolate_; } + bool HasReceivedSignal() { return received_signal_; } void HandleSigint(); private: void Destroy(); v8::Isolate* isolate_; + bool received_signal_; bool destroyed_; }; diff --git a/test/parallel/test-vm-timeout.js b/test/parallel/test-vm-timeout.js index d595bac4c3..0536ae37a1 100644 --- a/test/parallel/test-vm-timeout.js +++ b/test/parallel/test-vm-timeout.js @@ -32,3 +32,26 @@ assert.throws(function() { vm.runInNewContext('runInVM(10)', context, { timeout: 10000 }); throw new Error('Test 5 failed'); }, /Script execution timed out./); + +// Test 6: Nested vm timeouts, outer timeout is shorter and fires first. +assert.throws(function() { + const context = { + runInVM: function(timeout) { + vm.runInNewContext('while(true) {}', context, { timeout: timeout }); + } + }; + vm.runInNewContext('runInVM(10000)', context, { timeout: 100 }); + throw new Error('Test 6 failed'); +}, /Script execution timed out./); + +// Test 7: Nested vm timeouts, inner script throws an error. +assert.throws(function() { + const context = { + runInVM: function(timeout) { + vm.runInNewContext('throw new Error(\'foobar\')', context, { + timeout: timeout + }); + } + }; + vm.runInNewContext('runInVM(10000)', context, { timeout: 100000 }); +}, /foobar/);