Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 14e542e

Browse filesBrowse files
addaleaxFishrock123
addaleax
authored and
Fishrock123
committed
vm: test for abort condition of current invocation
When a vm script aborted after a timeout/signal interruption, test whether the local timeout/signal watchdog was responsible for terminating the execution. Without this, when a shorter timer from an outer `vm.run*` invocation fires before an inner timeout, the inner timeout would throw an error instead of the outer one, but because it did not witness the timeout itself, it would assume the termination was the result of a signal interruption. PR-URL: #7373 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
1 parent dad15cc commit 14e542e
Copy full SHA for 14e542e

File tree

Expand file treeCollapse file tree

4 files changed

+45
-4
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+45
-4
lines changed
Open diff view settings
Collapse file

‎src/node_contextify.cc‎

Copy file name to clipboardExpand all lines: src/node_contextify.cc
+18-3Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -836,14 +836,17 @@ class ContextifyScript : public BaseObject {
836836

837837
Local<Value> result;
838838
bool timed_out = false;
839+
bool received_signal = false;
839840
if (break_on_sigint && timeout != -1) {
840841
Watchdog wd(env->isolate(), timeout);
841842
SigintWatchdog swd(env->isolate());
842843
result = script->Run();
843844
timed_out = wd.HasTimedOut();
845+
received_signal = swd.HasReceivedSignal();
844846
} else if (break_on_sigint) {
845847
SigintWatchdog swd(env->isolate());
846848
result = script->Run();
849+
received_signal = swd.HasReceivedSignal();
847850
} else if (timeout != -1) {
848851
Watchdog wd(env->isolate(), timeout);
849852
result = script->Run();
@@ -852,14 +855,26 @@ class ContextifyScript : public BaseObject {
852855
result = script->Run();
853856
}
854857

855-
if (try_catch.HasCaught() && try_catch.HasTerminated()) {
856-
env->isolate()->CancelTerminateExecution();
858+
if (try_catch.HasCaught()) {
859+
if (try_catch.HasTerminated())
860+
env->isolate()->CancelTerminateExecution();
861+
862+
// It is possible that execution was terminated by another timeout in
863+
// which this timeout is nested, so check whether one of the watchdogs
864+
// from this invocation is responsible for termination.
857865
if (timed_out) {
858866
env->ThrowError("Script execution timed out.");
859-
} else {
867+
} else if (received_signal) {
860868
env->ThrowError("Script execution interrupted.");
861869
}
870+
871+
// If there was an exception thrown during script execution, re-throw it.
872+
// If one of the above checks threw, re-throw the exception instead of
873+
// letting try_catch catch it.
874+
// If execution has been terminated, but not by one of the watchdogs from
875+
// this invocation, this will re-throw a `null` value.
862876
try_catch.ReThrow();
877+
863878
return false;
864879
}
865880

Collapse file

‎src/node_watchdog.cc‎

Copy file name to clipboardExpand all lines: src/node_watchdog.cc
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ void SigintWatchdog::Dispose() {
9999

100100

101101
SigintWatchdog::SigintWatchdog(v8::Isolate* isolate)
102-
: isolate_(isolate), destroyed_(false) {
102+
: isolate_(isolate), received_signal_(false), destroyed_(false) {
103103
// Register this watchdog with the global SIGINT/Ctrl+C listener.
104104
SigintWatchdogHelper::GetInstance()->Register(this);
105105
// Start the helper thread, if that has not already happened.
@@ -120,6 +120,7 @@ void SigintWatchdog::Destroy() {
120120

121121

122122
void SigintWatchdog::HandleSigint() {
123+
received_signal_ = true;
123124
isolate_->TerminateExecution();
124125
}
125126

Collapse file

‎src/node_watchdog.h‎

Copy file name to clipboardExpand all lines: src/node_watchdog.h
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,13 @@ class SigintWatchdog {
4646
void Dispose();
4747

4848
v8::Isolate* isolate() { return isolate_; }
49+
bool HasReceivedSignal() { return received_signal_; }
4950
void HandleSigint();
5051
private:
5152
void Destroy();
5253

5354
v8::Isolate* isolate_;
55+
bool received_signal_;
5456
bool destroyed_;
5557
};
5658

Collapse file

‎test/parallel/test-vm-timeout.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-vm-timeout.js
+23Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,26 @@ assert.throws(function() {
3232
vm.runInNewContext('runInVM(10)', context, { timeout: 10000 });
3333
throw new Error('Test 5 failed');
3434
}, /Script execution timed out./);
35+
36+
// Test 6: Nested vm timeouts, outer timeout is shorter and fires first.
37+
assert.throws(function() {
38+
const context = {
39+
runInVM: function(timeout) {
40+
vm.runInNewContext('while(true) {}', context, { timeout: timeout });
41+
}
42+
};
43+
vm.runInNewContext('runInVM(10000)', context, { timeout: 100 });
44+
throw new Error('Test 6 failed');
45+
}, /Script execution timed out./);
46+
47+
// Test 7: Nested vm timeouts, inner script throws an error.
48+
assert.throws(function() {
49+
const context = {
50+
runInVM: function(timeout) {
51+
vm.runInNewContext('throw new Error(\'foobar\')', context, {
52+
timeout: timeout
53+
});
54+
}
55+
};
56+
vm.runInNewContext('runInVM(10000)', context, { timeout: 100000 });
57+
}, /foobar/);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.