Skip to content

Commit 07ad987

Browse files
jazellyaduh95
authored andcommittedOct 19, 2024
lib: convert transfer sequence to array in js
This commit lets `tranfer` passed to `structuredClone` get validated at JS layer by doing webidl conversion. This avoids the C++ to JS function call overhead in the native implementaiton of `structuredClone` PR-URL: #55317 Fixes: #55280 Refs: #50330 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
1 parent fe667be commit 07ad987

File tree

7 files changed

+89
-24
lines changed

7 files changed

+89
-24
lines changed
 

‎lib/internal/bootstrap/web/exposed-window-or-worker.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,11 @@ const {
3838
} = require('internal/process/task_queues');
3939
defineOperation(globalThis, 'queueMicrotask', queueMicrotask);
4040

41-
const { structuredClone } = internalBinding('messaging');
42-
defineOperation(globalThis, 'structuredClone', structuredClone);
41+
defineLazyProperties(
42+
globalThis,
43+
'internal/worker/js_transferable',
44+
['structuredClone'],
45+
);
4346
defineLazyProperties(globalThis, 'buffer', ['atob', 'btoa']);
4447

4548
// https://html.spec.whatwg.org/multipage/web-messaging.html#broadcasting-to-other-browsing-contexts

‎lib/internal/perf/usertiming.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ const {
3131
},
3232
} = require('internal/errors');
3333

34-
const { structuredClone } = internalBinding('messaging');
34+
const { structuredClone } = require('internal/worker/js_transferable');
3535
const {
3636
lazyDOMException,
3737
kEnumerableProperty,

‎lib/internal/webidl.js

+12
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,16 @@ converters.any = (V) => {
3838
return V;
3939
};
4040

41+
converters.object = (V, opts = kEmptyObject) => {
42+
if (type(V) !== 'Object') {
43+
throw makeException(
44+
'is not an object',
45+
kEmptyObject,
46+
);
47+
}
48+
return V;
49+
};
50+
4151
// https://webidl.spec.whatwg.org/#abstract-opdef-integerpart
4252
const integerPart = MathTrunc;
4353

@@ -189,6 +199,8 @@ converters.DOMString = function DOMString(V) {
189199
return String(V);
190200
};
191201

202+
converters['sequence<object>'] = createSequenceConverter(converters.object);
203+
192204
function codedTypeError(message, errorProperties = kEmptyObject) {
193205
// eslint-disable-next-line no-restricted-syntax
194206
const err = new TypeError(message);

‎lib/internal/webstreams/readablestream.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ const {
7474
kTransfer,
7575
kTransferList,
7676
markTransferMode,
77+
structuredClone,
7778
} = require('internal/worker/js_transferable');
7879

7980
const {
@@ -88,8 +89,6 @@ const {
8889
kControllerErrorFunction,
8990
} = require('internal/streams/utils');
9091

91-
const { structuredClone } = internalBinding('messaging');
92-
9392
const {
9493
ArrayBufferViewGetBuffer,
9594
ArrayBufferViewGetByteLength,

‎lib/internal/worker/js_transferable.js

+37
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,13 @@ const {
33
Error,
44
StringPrototypeSplit,
55
} = primordials;
6+
const {
7+
codes: {
8+
ERR_INVALID_ARG_TYPE,
9+
ERR_MISSING_ARGS,
10+
},
11+
} = require('internal/errors');
12+
const webidl = require('internal/webidl');
613
const {
714
messaging_deserialize_symbol,
815
messaging_transfer_symbol,
@@ -11,6 +18,7 @@ const {
1118
} = internalBinding('symbols');
1219
const {
1320
setDeserializerCreateObjectFunction,
21+
structuredClone: nativeStructuredClone,
1422
} = internalBinding('messaging');
1523
const {
1624
privateSymbols: {
@@ -90,9 +98,38 @@ function markTransferMode(obj, cloneable = false, transferable = false) {
9098
obj[transfer_mode_private_symbol] = mode;
9199
}
92100

101+
function structuredClone(value, options) {
102+
if (arguments.length === 0) {
103+
throw new ERR_MISSING_ARGS('The value argument must be specified');
104+
}
105+
106+
// TODO(jazelly): implement generic webidl dictionary converter
107+
const prefix = 'Options';
108+
const optionsType = webidl.type(options);
109+
if (optionsType !== 'Undefined' && optionsType !== 'Null' && optionsType !== 'Object') {
110+
throw new ERR_INVALID_ARG_TYPE(
111+
prefix,
112+
['object', 'null', 'undefined'],
113+
options,
114+
);
115+
}
116+
const key = 'transfer';
117+
const idlOptions = { __proto__: null, [key]: [] };
118+
if (options != null && key in options && options[key] !== undefined) {
119+
idlOptions[key] = webidl.converters['sequence<object>'](options[key], {
120+
__proto__: null,
121+
context: 'Transfer',
122+
});
123+
}
124+
125+
const serializedData = nativeStructuredClone(value, idlOptions);
126+
return serializedData;
127+
}
128+
93129
module.exports = {
94130
markTransferMode,
95131
setup,
132+
structuredClone,
96133
kClone: messaging_clone_symbol,
97134
kDeserialize: messaging_deserialize_symbol,
98135
kTransfer: messaging_transfer_symbol,

‎src/node_messaging.cc

+11-18
Original file line numberDiff line numberDiff line change
@@ -1578,28 +1578,21 @@ static void StructuredClone(const FunctionCallbackInfo<Value>& args) {
15781578
Realm* realm = Realm::GetCurrent(context);
15791579
Environment* env = realm->env();
15801580

1581-
if (args.Length() == 0) {
1582-
return THROW_ERR_MISSING_ARGS(env, "The value argument must be specified");
1583-
}
1584-
15851581
Local<Value> value = args[0];
15861582

15871583
TransferList transfer_list;
1588-
if (!args[1]->IsNullOrUndefined()) {
1589-
if (!args[1]->IsObject()) {
1590-
return THROW_ERR_INVALID_ARG_TYPE(
1591-
env, "The options argument must be either an object or undefined");
1592-
}
1593-
Local<Object> options = args[1].As<Object>();
1594-
Local<Value> transfer_list_v;
1595-
if (!options->Get(context, env->transfer_string())
1596-
.ToLocal(&transfer_list_v)) {
1597-
return;
1598-
}
1584+
Local<Object> options = args[1].As<Object>();
1585+
Local<Value> transfer_list_v;
1586+
if (!options->Get(context, env->transfer_string())
1587+
.ToLocal(&transfer_list_v)) {
1588+
return;
1589+
}
15991590

1600-
// TODO(joyeecheung): implement this in JS land to avoid the C++ -> JS
1601-
// cost to convert a sequence into an array.
1602-
if (!GetTransferList(env, context, transfer_list_v, &transfer_list)) {
1591+
Local<Array> arr = transfer_list_v.As<Array>();
1592+
size_t length = arr->Length();
1593+
transfer_list.AllocateSufficientStorage(length);
1594+
for (size_t i = 0; i < length; i++) {
1595+
if (!arr->Get(context, i).ToLocal(&transfer_list[i])) {
16031596
return;
16041597
}
16051598
}

‎test/parallel/test-structuredClone-global.js

+22-1
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@ assert.throws(() => structuredClone(undefined, ''), { code: 'ERR_INVALID_ARG_TYP
88
assert.throws(() => structuredClone(undefined, 1), { code: 'ERR_INVALID_ARG_TYPE' });
99
assert.throws(() => structuredClone(undefined, { transfer: 1 }), { code: 'ERR_INVALID_ARG_TYPE' });
1010
assert.throws(() => structuredClone(undefined, { transfer: '' }), { code: 'ERR_INVALID_ARG_TYPE' });
11+
assert.throws(() => structuredClone(undefined, { transfer: null }), { code: 'ERR_INVALID_ARG_TYPE' });
1112

1213
// Options can be null or undefined.
1314
assert.strictEqual(structuredClone(undefined), undefined);
1415
assert.strictEqual(structuredClone(undefined, null), undefined);
1516
// Transfer can be null or undefined.
16-
assert.strictEqual(structuredClone(undefined, { transfer: null }), undefined);
1717
assert.strictEqual(structuredClone(undefined, { }), undefined);
1818

1919
// Transferables or its subclasses should be received with its closest transferable superclass
@@ -43,6 +43,27 @@ for (const Transferrable of [File, Blob]) {
4343
assert.ok(extendedTransfer instanceof Transferrable);
4444
}
4545

46+
// Transfer can be iterable
47+
{
48+
const value = {
49+
a: new ReadableStream(),
50+
b: new WritableStream(),
51+
};
52+
const cloned = structuredClone(value, {
53+
transfer: {
54+
*[Symbol.iterator]() {
55+
for (const key in value) {
56+
yield value[key];
57+
}
58+
}
59+
}
60+
});
61+
for (const key in value) {
62+
assert.ok(value[key].locked);
63+
assert.ok(!cloned[key].locked);
64+
}
65+
}
66+
4667
{
4768
// See: https://github.com/nodejs/node/issues/49940
4869
const cloned = structuredClone({}, {

0 commit comments

Comments
 (0)