Skip to content

Commit

Permalink
lib: add support for JSTransferable as a mixin
Browse files Browse the repository at this point in the history
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 <[email protected]>

PR-URL: nodejs#38383
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Khaidi Chu <[email protected]>
  • Loading branch information
jasnell committed Apr 26, 2021
1 parent dee3741 commit bbe24e2
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 2 deletions.
42 changes: 42 additions & 0 deletions lib/internal/test/transfer.js
Original file line number Diff line number Diff line change
@@ -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 };
18 changes: 16 additions & 2 deletions lib/internal/worker/js_transferable.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
'use strict';
const {
Error,
ObjectDefineProperties,
ObjectGetOwnPropertyDescriptors,
ObjectGetPrototypeOf,
ObjectSetPrototypeOf,
ReflectConstruct,
StringPrototypeSplit,
} = primordials;
const {
Expand All @@ -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,
};
1 change: 1 addition & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
27 changes: 27 additions & 0 deletions test/parallel/test-messaging-maketransferable.js
Original file line number Diff line number Diff line change
@@ -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));

0 comments on commit bbe24e2

Please sign in to comment.