Skip to content

Commit

Permalink
doc, test: document and test vm timeout escapes
Browse files Browse the repository at this point in the history
Using `process.nextTick()` or `Promise`, it
is possible to escape the `timeout` set when running code with
`vm.runInContext()`, `vm.runInThisContext()`, and
`vm.runInNewContext()`.

This documents the issue and adds two known_issues tests.

Refs: #3020
PR-URL: #23743
Refs: #3020
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
  • Loading branch information
jasnell authored and MylesBorins committed Dec 26, 2018
1 parent 3170cb4 commit 6f3bc0d
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 0 deletions.
32 changes: 32 additions & 0 deletions doc/api/vm.md
Original file line number Diff line number Diff line change
Expand Up @@ -944,6 +944,38 @@ within which it can operate. The process of creating the V8 Context and
associating it with the `sandbox` object is what this document refers to as
"contextifying" the `sandbox`.

## Timeout limitations when using process.nextTick(), and Promises

Because of the internal mechanics of how the `process.nextTick()` queue and
the microtask queue that underlies Promises are implemented within V8 and
Node.js, it is possible for code running within a context to "escape" the
`timeout` set using `vm.runInContext()`, `vm.runInNewContext()`, and
`vm.runInThisContext()`.

For example, the following code executed by `vm.runInNewContext()` with a
timeout of 5 milliseconds schedules an infinite loop to run after a promise
resolves. The scheduled loop is never interrupted by the timeout:

```js
const vm = require('vm');

function loop() {
while (1) console.log(Date.now());
}

vm.runInNewContext(
'Promise.resolve().then(loop);',
{ loop, console },
{ timeout: 5 }
);
```

This issue also occurs when the `loop()` call is scheduled using
the `process.nextTick()` function.

This issue occurs because all contexts share the same microtask and nextTick
queues.

[`Error`]: errors.html#errors_class_error
[`URL`]: url.html#url_class_url
[`eval()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval
Expand Down
41 changes: 41 additions & 0 deletions test/known_issues/test-vm-timeout-escape-nexttick.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
'use strict';

// https://github.com/nodejs/node/issues/3020
// Promises, nextTick, and queueMicrotask allow code to escape the timeout
// set for runInContext, runInNewContext, and runInThisContext

require('../common');
const assert = require('assert');
const vm = require('vm');

const NS_PER_MS = 1000000n;

const hrtime = process.hrtime.bigint;
const nextTick = process.nextTick;

function loop() {
const start = hrtime();
while (1) {
const current = hrtime();
const span = (current - start) / NS_PER_MS;
if (span >= 100n) {
throw new Error(
`escaped timeout at ${span} milliseconds!`);
}
}
}

assert.throws(() => {
vm.runInNewContext(
'nextTick(loop); loop();',
{
hrtime,
nextTick,
loop
},
{ timeout: 5 }
);
}, {
code: 'ERR_SCRIPT_EXECUTION_TIMEOUT',
message: 'Script execution timed out after 5ms'
});
39 changes: 39 additions & 0 deletions test/known_issues/test-vm-timeout-escape-promise.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
'use strict';

// https://github.com/nodejs/node/issues/3020
// Promises, nextTick, and queueMicrotask allow code to escape the timeout
// set for runInContext, runInNewContext, and runInThisContext

require('../common');
const assert = require('assert');
const vm = require('vm');

const NS_PER_MS = 1000000n;

const hrtime = process.hrtime.bigint;

function loop() {
const start = hrtime();
while (1) {
const current = hrtime();
const span = (current - start) / NS_PER_MS;
if (span >= 100n) {
throw new Error(
`escaped timeout at ${span} milliseconds!`);
}
}
}

assert.throws(() => {
vm.runInNewContext(
'Promise.resolve().then(loop); loop();',
{
hrtime,
loop
},
{ timeout: 5 }
);
}, {
code: 'ERR_SCRIPT_EXECUTION_TIMEOUT',
message: 'Script execution timed out after 5ms'
});

0 comments on commit 6f3bc0d

Please sign in to comment.