Skip to content

Commit

Permalink
vm: add run-after-evaluate microtask mode
Browse files Browse the repository at this point in the history
This allows timeouts to apply to e.g. `Promise`s and `async function`s
from code running inside of `vm.Context`s, by giving the Context its
own microtasks queue.

Fixes: #3020

PR-URL: #34023
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
  • Loading branch information
addaleax authored and MylesBorins committed Jul 14, 2020
1 parent b3feaaf commit 7dffcb3
Show file tree
Hide file tree
Showing 13 changed files with 342 additions and 39 deletions.
74 changes: 63 additions & 11 deletions doc/api/vm.md
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,9 @@ overhead.
<!-- YAML
added: v0.3.1
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/34023
description: The `microtaskMode` option is supported now.
- version: v10.0.0
pr-url: https://github.com/nodejs/node/pull/19016
description: The `contextCodeGeneration` option is supported now.
Expand Down Expand Up @@ -225,6 +228,10 @@ changes:
`EvalError`. **Default:** `true`.
* `wasm` {boolean} If set to false any attempt to compile a WebAssembly
module will throw a `WebAssembly.CompileError`. **Default:** `true`.
* `microtaskMode` {string} If set to `afterEvaluate`, microtasks (tasks
scheduled through `Promise`s any `async function`s) will be run immediately
after the script has run. They are included in the `timeout` and
`breakOnSigint` scopes in that case.
* Returns: {any} the result of the very last statement executed in the script.

First contextifies the given `contextObject`, runs the compiled code contained
Expand Down Expand Up @@ -842,6 +849,9 @@ function with the given `params`.
<!-- YAML
added: v0.3.1
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/34023
description: The `microtaskMode` option is supported now.
- version: v10.0.0
pr-url: https://github.com/nodejs/node/pull/19398
description: The first argument can no longer be a function.
Expand All @@ -867,6 +877,10 @@ changes:
`EvalError`. **Default:** `true`.
* `wasm` {boolean} If set to false any attempt to compile a WebAssembly
module will throw a `WebAssembly.CompileError`. **Default:** `true`.
* `microtaskMode` {string} If set to `afterEvaluate`, microtasks (tasks
scheduled through `Promise`s any `async function`s) will be run immediately
after a script has run through [`script.runInContext()`][].
They are included in the `timeout` and `breakOnSigint` scopes in that case.
* Returns: {Object} contextified object.

If given a `contextObject`, the `vm.createContext()` method will [prepare
Expand Down Expand Up @@ -998,6 +1012,9 @@ console.log(contextObject);
<!-- YAML
added: v0.3.1
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/34023
description: The `microtaskMode` option is supported now.
- version: v10.0.0
pr-url: https://github.com/nodejs/node/pull/19016
description: The `contextCodeGeneration` option is supported now.
Expand Down Expand Up @@ -1064,6 +1081,10 @@ changes:
* Returns: {Module Namespace Object|vm.Module} Returning a `vm.Module` is
recommended in order to take advantage of error tracking, and to avoid
issues with namespaces that contain `then` function exports.
* `microtaskMode` {string} If set to `afterEvaluate`, microtasks (tasks
scheduled through `Promise`s any `async function`s) will be run immediately
after the script has run. They are included in the `timeout` and
`breakOnSigint` scopes in that case.
* Returns: {any} the result of the very last statement executed in the script.
The `vm.runInNewContext()` first contextifies the given `contextObject` (or
Expand Down Expand Up @@ -1220,13 +1241,13 @@ within which it can operate. The process of creating the V8 Context and
associating it with the `contextObject` is what this document refers to as
"contextifying" the object.

## Timeout limitations when using `process.nextTick()`, promises, and `queueMicrotask()`
## Timeout interactions with asynchronous tasks 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()`.
`Promise`s and `async function`s can schedule tasks run by the JavaScript
engine asynchronously. By default, these tasks are run after all JavaScript
functions on the current stack are done executing.
This allows escaping the functionality of the `timeout` and
`breakOnSigint` options.

For example, the following code executed by `vm.runInNewContext()` with a
timeout of 5 milliseconds schedules an infinite loop to run after a promise
Expand All @@ -1236,21 +1257,52 @@ resolves. The scheduled loop is never interrupted by the timeout:
const vm = require('vm');
function loop() {
console.log('entering loop');
while (1) console.log(Date.now());
}
vm.runInNewContext(
'Promise.resolve().then(loop);',
'Promise.resolve().then(() => loop());',
{ loop, console },
{ timeout: 5 }
);
// This prints *before* 'entering loop' (!)
console.log('done executing');
```

This issue also occurs when the `loop()` call is scheduled using
the `process.nextTick()` and `queueMicrotask()` functions.
This can be addressed by passing `microtaskMode: 'afterEvaluate'` to the code
that creates the `Context`:

This issue occurs because all contexts share the same microtask and nextTick
queues.
```js
const vm = require('vm');
function loop() {
while (1) console.log(Date.now());
}
vm.runInNewContext(
'Promise.resolve().then(() => loop());',
{ loop, console },
{ timeout: 5, microtaskMode: 'afterEvaluate' }
);
```

In this case, the microtask scheduled through `promise.then()` will be run
before returning from `vm.runInNewContext()`, and will be interrupted
by the `timeout` functionality. This applies only to code running in a
`vm.Context`, so e.g. [`vm.runInThisContext()`][] does not take this option.

Promise callbacks are entered into the microtask queue of the context in which
they were created. For example, if `() => loop()` is replaced with just `loop`
in the above example, then `loop` will be pushed into the global microtask
queue, because it is a function from the outer (main) context, and thus will
also be able to escape the timeout.

If asynchronous scheduling functions such as `process.nextTick()`,
`queueMicrotask()`, `setTimeout()`, `setImmediate()`, etc. are made available
inside a `vm.Context`, functions passed to them will be added to global queues,
which are shared by all contexts. Therefore, callbacks passed to those functions
are not controllable through the timeout either.

[`ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING`]: errors.html#ERR_VM_DYNAMIC_IMPORT_CALLBACK_MISSING
[`ERR_VM_MODULE_STATUS`]: errors.html#ERR_VM_MODULE_STATUS
Expand Down
24 changes: 22 additions & 2 deletions lib/vm.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const {

const {
ContextifyScript,
MicrotaskQueue,
makeContext,
isContext: _isContext,
constants,
Expand Down Expand Up @@ -186,6 +187,7 @@ function getContextOptions(options) {
name: options.contextName,
origin: options.contextOrigin,
codeGeneration: undefined,
microtaskMode: options.microtaskMode,
};
if (contextOptions.name !== undefined)
validateString(contextOptions.name, 'options.contextName');
Expand All @@ -201,6 +203,8 @@ function getContextOptions(options) {
validateBoolean(wasm, 'options.contextCodeGeneration.wasm');
contextOptions.codeGeneration = { strings, wasm };
}
if (options.microtaskMode !== undefined)
validateString(options.microtaskMode, 'options.microtaskMode');
return contextOptions;
}

Expand All @@ -222,7 +226,8 @@ function createContext(contextObject = {}, options = {}) {
const {
name = `VM Context ${defaultContextNameIndex++}`,
origin,
codeGeneration
codeGeneration,
microtaskMode
} = options;

validateString(name, 'options.name');
Expand All @@ -239,7 +244,22 @@ function createContext(contextObject = {}, options = {}) {
validateBoolean(wasm, 'options.codeGeneration.wasm');
}

makeContext(contextObject, name, origin, strings, wasm);
let microtaskQueue = null;
if (microtaskMode !== undefined) {
validateString(microtaskMode, 'options.microtaskMode');

if (microtaskMode === 'afterEvaluate') {
microtaskQueue = new MicrotaskQueue();
} else {
throw new ERR_INVALID_ARG_VALUE(
'options.microtaskQueue',
microtaskQueue,
'must be \'afterEvaluate\' or undefined'
);
}
}

makeContext(contextObject, name, origin, strings, wasm, microtaskQueue);
return contextObject;
}

Expand Down
1 change: 1 addition & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,7 @@ constexpr size_t kFsStatsBufferLength =
V(i18n_converter_template, v8::ObjectTemplate) \
V(libuv_stream_wrap_ctor_template, v8::FunctionTemplate) \
V(message_port_constructor_template, v8::FunctionTemplate) \
V(microtask_queue_ctor_template, v8::FunctionTemplate) \
V(pipe_constructor_template, v8::FunctionTemplate) \
V(promise_wrap_template, v8::ObjectTemplate) \
V(sab_lifetimepartner_constructor_template, v8::FunctionTemplate) \
Expand Down
31 changes: 22 additions & 9 deletions src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ using v8::IntegrityLevel;
using v8::Isolate;
using v8::Local;
using v8::MaybeLocal;
using v8::MicrotaskQueue;
using v8::Module;
using v8::Number;
using v8::Object;
Expand Down Expand Up @@ -106,15 +107,15 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
Local<String> url = args[0].As<String>();

Local<Context> context;
ContextifyContext* contextify_context = nullptr;
if (args[1]->IsUndefined()) {
context = that->CreationContext();
} else {
CHECK(args[1]->IsObject());
ContextifyContext* sandbox =
ContextifyContext::ContextFromContextifiedSandbox(
env, args[1].As<Object>());
CHECK_NOT_NULL(sandbox);
context = sandbox->context();
contextify_context = ContextifyContext::ContextFromContextifiedSandbox(
env, args[1].As<Object>());
CHECK_NOT_NULL(contextify_context);
context = contextify_context->context();
}

Local<Integer> line_offset;
Expand Down Expand Up @@ -224,6 +225,7 @@ void ModuleWrap::New(const FunctionCallbackInfo<Value>& args) {
}

obj->context_.Reset(isolate, context);
obj->contextify_context_ = contextify_context;

env->hash_to_module_map.emplace(module->GetIdentityHash(), obj);

Expand Down Expand Up @@ -319,6 +321,11 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
Local<Context> context = obj->context_.Get(isolate);
Local<Module> module = obj->module_.Get(isolate);

ContextifyContext* contextify_context = obj->contextify_context_;
std::shared_ptr<MicrotaskQueue> microtask_queue;
if (contextify_context != nullptr)
microtask_queue = contextify_context->microtask_queue();

// module.evaluate(timeout, breakOnSigint)
CHECK_EQ(args.Length(), 2);

Expand All @@ -334,18 +341,24 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
bool timed_out = false;
bool received_signal = false;
MaybeLocal<Value> result;
auto run = [&]() {
MaybeLocal<Value> result = module->Evaluate(context);
if (!result.IsEmpty() && microtask_queue)
microtask_queue->PerformCheckpoint(isolate);
return result;
};
if (break_on_sigint && timeout != -1) {
Watchdog wd(isolate, timeout, &timed_out);
SigintWatchdog swd(isolate, &received_signal);
result = module->Evaluate(context);
result = run();
} else if (break_on_sigint) {
SigintWatchdog swd(isolate, &received_signal);
result = module->Evaluate(context);
result = run();
} else if (timeout != -1) {
Watchdog wd(isolate, timeout, &timed_out);
result = module->Evaluate(context);
result = run();
} else {
result = module->Evaluate(context);
result = run();
}

if (result.IsEmpty()) {
Expand Down
9 changes: 7 additions & 2 deletions src/module_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ namespace node {

class Environment;

namespace contextify {
class ContextifyContext;
}

namespace loader {

enum ScriptType : int {
Expand Down Expand Up @@ -82,12 +86,13 @@ class ModuleWrap : public BaseObject {
static ModuleWrap* GetFromModule(node::Environment*, v8::Local<v8::Module>);

v8::Global<v8::Function> synthetic_evaluation_steps_;
bool synthetic_ = false;
v8::Global<v8::Module> module_;
v8::Global<v8::String> url_;
bool linked_ = false;
std::unordered_map<std::string, v8::Global<v8::Promise>> resolve_cache_;
v8::Global<v8::Context> context_;
contextify::ContextifyContext* contextify_context_ = nullptr;
bool synthetic_ = false;
bool linked_ = false;
uint32_t id_;
};

Expand Down
Loading

0 comments on commit 7dffcb3

Please sign in to comment.