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

worker: implement worker.moveMessagePortToContext() #26497

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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions doc/api/worker_threads.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,30 @@ if (isMainThread) {
}
```

## worker.moveMessagePortToContext(port, contextifiedSandbox)
<!-- YAML
added: REPLACEME
-->

* `port` {MessagePort} The message port which will be transferred.
* `contextifiedSandbox` {Object} A [contextified][] object as returned by the
`vm.createContext()` method.

* Returns: {MessagePort}

Transfer a `MessagePort` to a different [`vm`][] Context. The original `port`
object will be rendered unusable, and the returned `MessagePort` instance will
take its place.

The returned `MessagePort` will be an object in the target context, and will
inherit from its global `Object` class. Objects passed to the
[`port.onmessage()`][] listener will also be created in the target context
and inherit from its global `Object` class.

However, the created `MessagePort` will no longer inherit from
[`EventEmitter`][], and only [`port.onmessage()`][] can be used to receive
events using it.

## worker.parentPort
<!-- YAML
added: v10.5.0
Expand Down Expand Up @@ -583,6 +607,7 @@ active handle in the event system. If the worker is already `unref()`ed calling
[`Worker`]: #worker_threads_class_worker
[`cluster` module]: cluster.html
[`port.on('message')`]: #worker_threads_event_message
[`port.onmessage()`]: https://developer.mozilla.org/en-US/docs/Web/API/MessagePort/onmessage
[`port.postMessage()`]: #worker_threads_port_postmessage_value_transferlist
[`process.abort()`]: process.html#process_process_abort
[`process.chdir()`]: process.html#process_process_chdir_directory
Expand All @@ -600,6 +625,7 @@ active handle in the event system. If the worker is already `unref()`ed calling
[`require('worker_threads').threadId`]: #worker_threads_worker_threadid
[`require('worker_threads').workerData`]: #worker_threads_worker_workerdata
[`trace_events`]: tracing.html
[`vm`]: vm.html
[`worker.on('message')`]: #worker_threads_event_message_1
[`worker.postMessage()`]: #worker_threads_worker_postmessage_value_transferlist
[`worker.terminate()`]: #worker_threads_worker_terminate_callback
Expand All @@ -610,4 +636,5 @@ active handle in the event system. If the worker is already `unref()`ed calling
[Web Workers]: https://developer.mozilla.org/en-US/docs/Web/API/Web_Workers_API
[browser `MessagePort`]: https://developer.mozilla.org/en-US/docs/Web/API/MessagePort
[child processes]: child_process.html
[contextified]: vm.html#vm_what_does_it_mean_to_contextify_an_object
[v8.serdes]: v8.html#v8_serialization_api
6 changes: 4 additions & 2 deletions lib/internal/bootstrap/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@ const cannotBeRequired = [

'internal/test/binding',

'internal/bootstrap/context',
'internal/bootstrap/primordials',
'internal/bootstrap/loaders',
'internal/bootstrap/node'
'internal/bootstrap/node',

'internal/per_context/setup',
'internal/per_context/domexception',
];

// Skip modules that cannot be required when they are not
Expand Down
9 changes: 0 additions & 9 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,6 @@ if (!config.noBrowserGlobals) {
defineOperation(global, 'setImmediate', timers.setImmediate);
}

setupDOMException();

// process.allowedNodeEnvironmentFlags
Object.defineProperty(process, 'allowedNodeEnvironmentFlags', {
get() {
Expand Down Expand Up @@ -394,13 +392,6 @@ function createGlobalConsole(consoleFromVM) {
return consoleFromNode;
}

function setupDOMException() {
// Registers the constructor with C++.
const DOMException = NativeModule.require('internal/domexception');
const { registerDOMException } = internalBinding('messaging');
registerDOMException(DOMException);
}

// https://heycam.github.io/webidl/#es-namespaces
function exposeNamespace(target, name, namespaceObject) {
Object.defineProperty(target, name, {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
'use strict';

const { ERR_INVALID_THIS } = require('internal/errors').codes;
class ERR_INVALID_THIS extends TypeError {
constructor(type) {
super('Value of "this" must be of ' + type);
}

get code() { return 'ERR_INVALID_THIS'; }
}

const internalsMap = new WeakMap();

Expand Down Expand Up @@ -83,4 +89,4 @@ for (const [name, codeName, value] of [
nameToCodeMap.set(name, value);
}

module.exports = DOMException;
exports.DOMException = DOMException;
File renamed without changes.
2 changes: 2 additions & 0 deletions lib/internal/worker/io.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const {
MessagePort,
MessageChannel,
drainMessagePort,
moveMessagePortToContext,
stopMessagePort
} = internalBinding('messaging');
const { threadId } = internalBinding('worker');
Expand Down Expand Up @@ -233,6 +234,7 @@ module.exports = {
kIncrementsPortRef,
kWaitingStreams,
kStdioWantsMoreDataCallback,
moveMessagePortToContext,
MessagePort,
MessageChannel,
setupPortReferencing,
Expand Down
4 changes: 3 additions & 1 deletion lib/worker_threads.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ const {

const {
MessagePort,
MessageChannel
MessageChannel,
moveMessagePortToContext,
} = require('internal/worker/io');

module.exports = {
isMainThread,
MessagePort,
MessageChannel,
moveMessagePortToContext,
threadId,
Worker,
parentPort: null,
Expand Down
4 changes: 2 additions & 2 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,13 @@
'node_lib_target_name%': 'node_lib',
'node_intermediate_lib_type%': 'static_library',
'library_files': [
'lib/internal/bootstrap/context.js',
'lib/internal/bootstrap/primordials.js',
'lib/internal/bootstrap/cache.js',
'lib/internal/bootstrap/loaders.js',
'lib/internal/bootstrap/node.js',
'lib/internal/bootstrap/pre_execution.js',
'lib/internal/per_context/setup.js',
'lib/internal/per_context/domexception.js',
'lib/async_hooks.js',
'lib/assert.js',
'lib/buffer.js',
Expand Down Expand Up @@ -115,7 +116,6 @@
'lib/internal/dgram.js',
'lib/internal/dns/promises.js',
'lib/internal/dns/utils.js',
'lib/internal/domexception.js',
'lib/internal/dtrace.js',
'lib/internal/encoding.js',
'lib/internal/errors.js',
Expand Down
74 changes: 57 additions & 17 deletions src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,17 @@

namespace node {
using v8::Context;
using v8::EscapableHandleScope;
using v8::Function;
using v8::HandleScope;
using v8::Isolate;
using v8::Local;
using v8::MaybeLocal;
using v8::Message;
using v8::MicrotasksPolicy;
using v8::Object;
using v8::ObjectTemplate;
using v8::Private;
using v8::String;
using v8::Value;

Expand Down Expand Up @@ -279,6 +282,26 @@ void FreePlatform(MultiIsolatePlatform* platform) {
delete platform;
}

MaybeLocal<Object> GetPerContextExports(Local<Context> context) {
Isolate* isolate = context->GetIsolate();
EscapableHandleScope handle_scope(isolate);

Local<Object> global = context->Global();
Local<Private> key = Private::ForApi(isolate,
FIXED_ONE_BYTE_STRING(isolate, "node:per_context_binding_exports"));

Local<Value> existing_value;
if (!global->GetPrivate(context, key).ToLocal(&existing_value))
Copy link
Member

@joyeecheung joyeecheung Mar 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use context->GetExtrasBindingObject() to store these somehow instead of using v8 privates (which are...an experimental feature. Use at your own risk.)? V8 already puts isTraceCategoryEnabled and trace in there.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We’ve been using V8 privates extensively for a while, I don’t think they’re going away (and if they did we’d have bigger problems, e.g. N-API relies on them as well).

But yes, if you prefer, we could use the extras binding object for that … I feel like that’s mostly intended for actual v8-extras, but in theory nothing stops us, and I don’t mind making the switch if you prefer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@addaleax I don't have very strong opinions about the choices here...although I wonder what does this look like in the heap snapshot? Do they show up?

BTW, we probably need to do something similar for the Environment persistent handles (save them temporarily to the global proxy) when we implement snapshots...

return MaybeLocal<Object>();
if (existing_value->IsObject())
return handle_scope.Escape(existing_value.As<Object>());

Local<Object> exports = Object::New(isolate);
if (context->Global()->SetPrivate(context, key, exports).IsNothing())
return MaybeLocal<Object>();
return handle_scope.Escape(exports);
}

Local<Context> NewContext(Isolate* isolate,
Local<ObjectTemplate> object_template) {
auto context = Context::New(isolate, nullptr, object_template);
Expand All @@ -289,25 +312,42 @@ Local<Context> NewContext(Isolate* isolate,
True(isolate));

{
// Run lib/internal/bootstrap/context.js
// Run per-context JS files.
Context::Scope context_scope(context);

std::vector<Local<String>> parameters = {
FIXED_ONE_BYTE_STRING(isolate, "global")};
Local<Value> arguments[] = {context->Global()};
MaybeLocal<Function> maybe_fn =
per_process::native_module_loader.LookupAndCompile(
context, "internal/bootstrap/context", &parameters, nullptr);
if (maybe_fn.IsEmpty()) {
return Local<Context>();
}
Local<Function> fn = maybe_fn.ToLocalChecked();
MaybeLocal<Value> result =
fn->Call(context, Undefined(isolate), arraysize(arguments), arguments);
// Execution failed during context creation.
// TODO(joyeecheung): deprecate this signature and return a MaybeLocal.
if (result.IsEmpty()) {
Local<Object> exports;
if (!GetPerContextExports(context).ToLocal(&exports))
return Local<Context>();

Local<String> global_string = FIXED_ONE_BYTE_STRING(isolate, "global");
Local<String> exports_string = FIXED_ONE_BYTE_STRING(isolate, "exports");

static const char* context_files[] = {
"internal/per_context/setup",
"internal/per_context/domexception",
nullptr
};

for (const char** module = context_files; *module != nullptr; module++) {
std::vector<Local<String>> parameters = {
global_string,
exports_string
};
Local<Value> arguments[] = {context->Global(), exports};
MaybeLocal<Function> maybe_fn =
per_process::native_module_loader.LookupAndCompile(
context, *module, &parameters, nullptr);
if (maybe_fn.IsEmpty()) {
return Local<Context>();
}
Local<Function> fn = maybe_fn.ToLocalChecked();
MaybeLocal<Value> result =
fn->Call(context, Undefined(isolate),
arraysize(arguments), arguments);
// Execution failed during context creation.
// TODO(joyeecheung): deprecate this signature and return a MaybeLocal.
if (result.IsEmpty()) {
return Local<Context>();
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,8 @@ void DefineZlibConstants(v8::Local<v8::Object> target);
v8::MaybeLocal<v8::Value> RunBootstrapping(Environment* env);
v8::MaybeLocal<v8::Value> StartExecution(Environment* env,
const char* main_script_id);
v8::MaybeLocal<v8::Object> GetPerContextExports(v8::Local<v8::Context> context);

namespace profiler {
void StartCoverageCollection(Environment* env);
}
Expand Down
Loading