From f2c0258b4c62be38ff1d43a2e26995b69cb9f225 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 24 Apr 2021 07:46:21 -0700 Subject: [PATCH] lib: add support for JSTransferable as a mixin Adds a new `makeTransferable()` utility that can construct a `JSTransferable` object that does not directly extend the `JSTransferable` JavaScript class. Because JavaScript does not support multiple inheritance, it is not possible (without help) to implement a class that extends both `JSTransferable` and, for instance, `EventTarget` without incurring a significant additional complexity and performance cost by making all `EventTarget` instances extend `JSTransferable`... That is, we *don't* want: ```js class EventTarget extends JSTransferable { ... } ``` The `makeTransferable()` allows us to create objects that are backed internally by `JSTransferable` without having to actually extend it by leveraging the magic of `Reflect.construct()`. ```js const { JSTransferable, kClone, kDeserialize, kConstructor, makeTransferable, } = require('internal/worker/js_transferable'); class E { constructor(b) { this.b = b; } } class F extends E { [kClone]() { /** ... **/ } [kDeserialize]() { /** ... **/ } static [kConstructor]() { return makeTransferable(F); } } const f = makeTransferable(F, 1); f instanceof F; // true f instanceof E; // true f instanceof JSTransferable; // false const mc = new MessageChannel(); mc.port1.onmessage = ({ data }) => { data instanceof F; // true data instanceof E; // true data instanceof JSTransferable; // false }; mc.port2.postMessage(f); // works! ``` The additional `internal/test/transfer.js` file is required for the test because successfully deserializing transferable classes requires that they be located in `lib/internal` for now. Signed-off-by: James M Snell PR-URL: https://github.com/nodejs/node/pull/38383 Reviewed-By: Anna Henningsen Reviewed-By: Khaidi Chu --- lib/internal/test/transfer.js | 42 +++++++++++++++++++ lib/internal/worker/js_transferable.js | 18 +++++++- node.gyp | 1 + .../test-messaging-maketransferable.js | 27 ++++++++++++ 4 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 lib/internal/test/transfer.js create mode 100644 test/parallel/test-messaging-maketransferable.js diff --git a/lib/internal/test/transfer.js b/lib/internal/test/transfer.js new file mode 100644 index 00000000000000..b814c37fe6dc6d --- /dev/null +++ b/lib/internal/test/transfer.js @@ -0,0 +1,42 @@ +'use strict'; + +const { + makeTransferable, + kClone, + kDeserialize, +} = require('internal/worker/js_transferable'); + +process.emitWarning( + 'These APIs are for internal testing only. Do not use them.', + 'internal/test/transfer'); + +// Used as part of parallel/test-messaging-maketransferable. +// This has to exist within the lib/internal/ path in order +// for deserialization to work. + +class E { + constructor(b) { + this.b = b; + } +} + +class F extends E { + constructor(b) { + super(b); + /* eslint-disable-next-line no-constructor-return */ + return makeTransferable(this); + } + + [kClone]() { + return { + data: { b: this.b }, + deserializeInfo: 'internal/test/transfer:F' + }; + } + + [kDeserialize]({ b }) { + this.b = b; + } +} + +module.exports = { E, F }; diff --git a/lib/internal/worker/js_transferable.js b/lib/internal/worker/js_transferable.js index ce95cf64e21987..7bd6c8cafc32e2 100644 --- a/lib/internal/worker/js_transferable.js +++ b/lib/internal/worker/js_transferable.js @@ -1,6 +1,11 @@ 'use strict'; const { Error, + ObjectDefineProperties, + ObjectGetOwnPropertyDescriptors, + ObjectGetPrototypeOf, + ObjectSetPrototypeOf, + ReflectConstruct, StringPrototypeSplit, } = primordials; const { @@ -22,21 +27,30 @@ function setup() { const { 0: module, 1: ctor } = StringPrototypeSplit(deserializeInfo, ':'); const Ctor = require(module)[ctor]; if (typeof Ctor !== 'function' || - !(Ctor.prototype instanceof JSTransferable)) { + typeof Ctor.prototype[messaging_deserialize_symbol] !== 'function') { // Not one of the official errors because one should not be able to get // here without messing with Node.js internals. // eslint-disable-next-line no-restricted-syntax throw new Error(`Unknown deserialize spec ${deserializeInfo}`); } + return new Ctor(); }); } +function makeTransferable(obj) { + const inst = ReflectConstruct(JSTransferable, [], obj.constructor); + ObjectDefineProperties(inst, ObjectGetOwnPropertyDescriptors(obj)); + ObjectSetPrototypeOf(inst, ObjectGetPrototypeOf(obj)); + return inst; +} + module.exports = { + makeTransferable, setup, JSTransferable, kClone: messaging_clone_symbol, kDeserialize: messaging_deserialize_symbol, kTransfer: messaging_transfer_symbol, - kTransferList: messaging_transfer_list_symbol + kTransferList: messaging_transfer_list_symbol, }; diff --git a/node.gyp b/node.gyp index 54ede7fe785ad1..3308ecb432c34f 100644 --- a/node.gyp +++ b/node.gyp @@ -229,6 +229,7 @@ 'lib/internal/source_map/source_map.js', 'lib/internal/source_map/source_map_cache.js', 'lib/internal/test/binding.js', + 'lib/internal/test/transfer.js', 'lib/internal/timers.js', 'lib/internal/tls.js', 'lib/internal/trace_events_async_hooks.js', diff --git a/test/parallel/test-messaging-maketransferable.js b/test/parallel/test-messaging-maketransferable.js new file mode 100644 index 00000000000000..07b1081045d99f --- /dev/null +++ b/test/parallel/test-messaging-maketransferable.js @@ -0,0 +1,27 @@ +// Flags: --expose-internals +'use strict'; + +const common = require('../common'); + +const assert = require('assert'); +const { + JSTransferable, +} = require('internal/worker/js_transferable'); +const { E, F } = require('internal/test/transfer'); + +// Tests that F is transferable even tho it does not directly, +// observably extend the JSTransferable class. + +const mc = new MessageChannel(); + +mc.port1.onmessageerror = common.mustNotCall(); + +mc.port1.onmessage = common.mustCall(({ data }) => { + assert(!(data instanceof JSTransferable)); + assert(data instanceof F); + assert(data instanceof E); + assert.strictEqual(data.b, 1); + mc.port1.close(); +}); + +mc.port2.postMessage(new F(1));