vm: fix nested timeouts with inverse order + fix flaky test-vm-timeout

src/node_contextify.cc

@@ -836,14 +836,17 @@ class ContextifyScript : public BaseObject {
 
     Local<Value> 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;
     }
 

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();
 }
 

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_;
 };
 

test/parallel/parallel.status

@@ -7,8 +7,7 @@ prefix parallel
 [true] # This section applies to all platforms
 
 [$system==win32]
-test-tick-processor : PASS,FLAKY
-test-vm-timeout     : PASS,FLAKY
+test-tick-processor     : PASS,FLAKY
 
 [$system==linux]
 test-tick-processor           : PASS,FLAKY

test/parallel/test-vm-timeout.js

@@ -29,6 +29,29 @@ assert.throws(function() {
       vm.runInNewContext('while(true) {}', context, { timeout: timeout });
     }
   };
-  vm.runInNewContext('runInVM(10)', context, { timeout: 100 });
+  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/);