Skip to content

Commit

Permalink
worker: reduce MessagePort prototype to documented API
Browse files Browse the repository at this point in the history
`MessagePort` is special because it has to be a C++ API
that is exposed to userland. Therefore, there is a number
of internal methods on its native prototype; this commit
reduces this set of methods to only what is documented in
the API.

PR-URL: #23037
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
addaleax authored and targos committed Sep 27, 2018
1 parent 2e30a68 commit c34db7a
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 24 deletions.
49 changes: 31 additions & 18 deletions lib/internal/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,18 @@ const {

const { internalBinding } = require('internal/bootstrap/loaders');
const { MessagePort, MessageChannel } = internalBinding('messaging');
const { handle_onclose } = internalBinding('symbols');
const {
handle_onclose: handleOnCloseSymbol,
oninit: onInitSymbol
} = internalBinding('symbols');
const { clearAsyncIdStack } = require('internal/async_hooks');
const { serializeError, deserializeError } = require('internal/error-serdes');
const { pathToFileURL } = require('url');

util.inherits(MessagePort, EventEmitter);

const {
Worker: WorkerImpl,
getEnvMessagePort,
threadId,
oninit: oninit_symbol
threadId
} = internalBinding('worker');

const isMainThread = threadId === 0;
Expand Down Expand Up @@ -58,6 +58,23 @@ const messageTypes = {
LOAD_SCRIPT: 'loadScript'
};

// We have to mess with the MessagePort prototype a bit, so that a) we can make
// it inherit from EventEmitter, even though it is a C++ class, and b) we do
// not provide methods that are not present in the Browser and not documented
// on our side (e.g. hasRef).
// Save a copy of the original set of methods as a shallow clone.
const MessagePortPrototype = Object.create(
Object.getPrototypeOf(MessagePort.prototype),
Object.getOwnPropertyDescriptors(MessagePort.prototype));
// Set up the new inheritance chain.
Object.setPrototypeOf(MessagePort, EventEmitter);
Object.setPrototypeOf(MessagePort.prototype, EventEmitter.prototype);
// Finally, purge methods we don't want to be public.
delete MessagePort.prototype.stop;
delete MessagePort.prototype.drain;
delete MessagePort.prototype.hasRef;
delete MessagePort.prototype.getAsyncId;

// A communication channel consisting of a handle (that wraps around an
// uv_async_t) which can receive information from other threads and emits
// .onmessage events, and a function used for sending data to a MessagePort
Expand All @@ -81,10 +98,10 @@ Object.defineProperty(MessagePort.prototype, 'onmessage', {
this[kOnMessageListener] = value;
if (typeof value === 'function') {
this.ref();
this.start();
MessagePortPrototype.start.call(this);
} else {
this.unref();
this.stop();
MessagePortPrototype.stop.call(this);
}
}
});
Expand All @@ -94,7 +111,7 @@ function oninit() {
setupPortReferencing(this, this, 'message');
}

Object.defineProperty(MessagePort.prototype, oninit_symbol, {
Object.defineProperty(MessagePort.prototype, onInitSymbol, {
enumerable: true,
writable: false,
value: oninit
Expand All @@ -111,22 +128,18 @@ function onclose() {
this.emit('close');
}

Object.defineProperty(MessagePort.prototype, handle_onclose, {
Object.defineProperty(MessagePort.prototype, handleOnCloseSymbol, {
enumerable: false,
writable: false,
value: onclose
});

const originalClose = MessagePort.prototype.close;
MessagePort.prototype.close = function(cb) {
if (typeof cb === 'function')
this.once('close', cb);
originalClose.call(this);
MessagePortPrototype.close.call(this);
};

const drainMessagePort = MessagePort.prototype.drain;
delete MessagePort.prototype.drain;

Object.defineProperty(MessagePort.prototype, util.inspect.custom, {
enumerable: false,
writable: false,
Expand All @@ -135,7 +148,7 @@ Object.defineProperty(MessagePort.prototype, util.inspect.custom, {
try {
// This may throw when `this` does not refer to a native object,
// e.g. when accessing the prototype directly.
ref = this.hasRef();
ref = MessagePortPrototype.hasRef.call(this);
} catch { return this; }
return Object.assign(Object.create(MessagePort.prototype),
ref === undefined ? {
Expand All @@ -157,12 +170,12 @@ function setupPortReferencing(port, eventEmitter, eventName) {
eventEmitter.on('newListener', (name) => {
if (name === eventName && eventEmitter.listenerCount(eventName) === 0) {
port.ref();
port.start();
MessagePortPrototype.start.call(port);
}
});
eventEmitter.on('removeListener', (name) => {
if (name === eventName && eventEmitter.listenerCount(eventName) === 0) {
port.stop();
MessagePortPrototype.stop.call(port);
port.unref();
}
});
Expand Down Expand Up @@ -304,7 +317,7 @@ class Worker extends EventEmitter {

[kOnExit](code) {
debug(`[${threadId}] hears end event for Worker ${this.threadId}`);
drainMessagePort.call(this[kPublicPort]);
MessagePortPrototype.drain.call(this[kPublicPort]);
this[kDispose]();
this.emit('exit', code);
this.removeAllListeners();
Expand Down
4 changes: 0 additions & 4 deletions src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -501,10 +501,6 @@ void InitWorker(Local<Object> target,
thread_id_string,
Number::New(env->isolate(),
static_cast<double>(env->thread_id()))).FromJust();
Local<String> oninit_string = FIXED_ONE_BYTE_STRING(env->isolate(), "oninit");
target->Set(env->context(),
oninit_string,
env->oninit_symbol()).FromJust();
}

} // anonymous namespace
Expand Down
3 changes: 1 addition & 2 deletions test/parallel/test-heapdump-worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ validateSnapshotNodes('Worker', [
validateSnapshotNodes('MessagePort', [
{
children: [
{ name: 'MessagePortData' },
{ name: 'MessagePort' }
{ name: 'MessagePortData' }
]
}
], { loose: true });
Expand Down
9 changes: 9 additions & 0 deletions test/parallel/test-worker-message-port.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,12 @@ const { MessageChannel, MessagePort } = require('worker_threads');
});
});
}

{
assert.deepStrictEqual(
Object.getOwnPropertyNames(MessagePort.prototype).sort(),
[
'close', 'constructor', 'onmessage', 'postMessage', 'ref', 'start',
'unref'
]);
}

0 comments on commit c34db7a

Please sign in to comment.