Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vm: add run-after-evaluate microtask mode #34023

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
74 changes: 63 additions & 11 deletions doc/api/vm.md
Expand Up @@ -186,6 +186,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 @@ -223,6 +226,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 @@ -844,6 +851,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 @@ -869,6 +879,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 @@ -1000,6 +1014,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 @@ -1066,6 +1083,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 @@ -1222,13 +1243,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 @@ -1238,21 +1259,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
18 changes: 16 additions & 2 deletions lib/vm.js
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');
addaleax marked this conversation as resolved.
Show resolved Hide resolved
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,16 @@ 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();
}
}

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

Expand Down
1 change: 1 addition & 0 deletions src/env.h
Expand Up @@ -433,6 +433,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
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
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