FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

worker: add public method for marking objects as untransferable · nodejs/node@4e42eb5 · GitHub

/ node Public

Commit 4e42eb5

Browse files
addaleax
authored and
codebytere
committed
worker: add public method for marking objects as untransferable
We currently mark a number of `ArrayBuffer`s as not transferable, including the `Buffer` pool and ones with finalizers provided by C++ addons. There is no good reason to assume that userland code might not encounter similar problems, for example when doing `ArrayBuffer` pooling similar to ours. Therefore, provide an API that lets userland code also mark objects as not transferable. PR-URL: #33979 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gus Caplan <me@gus.host>
1 parent 74f4aae commit 4e42eb5

9 files changed

Lines changed: 118 additions & 21 deletions

File tree

‎doc/api/worker_threads.md‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,42 @@ if (isMainThread) {
8080
}
8181
```
8282

83+
## `worker.markAsUntransferable(object)`
84+
<!-- YAML
85+
added: REPLACEME
86+
-->
87+
88+
Mark an object as not transferable. If `object` occurs in the transfer list of
89+
a [`port.postMessage()`][] call, it will be ignored.
90+
91+
In particular, this makes sense for objects that can be cloned, rather than
92+
transferred, and which are used by other objects on the sending side.
93+
For example, Node.js marks the `ArrayBuffer`s it uses for its
94+
[`Buffer` pool][`Buffer.allocUnsafe()`] with this.
95+
96+
This operation cannot be undone.
97+
98+
```js
99+
const { MessageChannel, markAsUntransferable } = require('worker_threads');
100+
101+
const pooledBuffer = new ArrayBuffer(8);
102+
const typedArray1 = new Uint8Array(pooledBuffer);
103+
const typedArray2 = new Float64Array(pooledBuffer);
104+
105+
markAsUntransferable(pooledBuffer);
106+
107+
const { port1 } = new MessageChannel();
108+
port1.postMessage(typedArray1, [ typedArray1.buffer ]);
109+
110+
// The following line prints the contents of typedArray1 -- it still owns its
111+
// memory and has been cloned, not transfered. Without `markAsUntransferable()`,
112+
// this would print an empty Uint8Array. typedArray2 is intact as well.
113+
console.log(typedArray1);
114+
console.log(typedArray2);
115+
```
116+
117+
There is no equivalent to this API in browsers.
118+
83119
## `worker.moveMessagePortToContext(port, contextifiedSandbox)`
84120
<!-- YAML
85121
added: v11.13.0
@@ -442,6 +478,9 @@ For `Buffer` instances, specifically, whether the underlying
442478
`ArrayBuffer` can be transferred or cloned depends entirely on how
443479
instances were created, which often cannot be reliably determined.
444480

481+
An `ArrayBuffer` can be marked with [`markAsUntransferable()`][] to indicate
482+
that it should always be cloned and never transferred.
483+
445484
Depending on how a `Buffer` instance was created, it may or may
446485
not own its underlying `ArrayBuffer`. An `ArrayBuffer` must not
447486
be transferred unless it is known that the `Buffer` instance
@@ -853,6 +892,7 @@ active handle in the event system. If the worker is already `unref()`ed calling
853892
[`WebAssembly.Module`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WebAssembly/Module
854893
[`Worker`]: #worker_threads_class_worker
855894
[`cluster` module]: cluster.html
895+
[`markAsUntransferable()`]: #worker_threads_worker_markasuntransferable_object
856896
[`port.on('message')`]: #worker_threads_event_message
857897
[`port.onmessage()`]: https://developer.mozilla.org/en-US/docs/Web/API/MessagePort/onmessage
858898
[`port.postMessage()`]: #worker_threads_port_postmessage_value_transferlist

‎lib/buffer.js‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,11 @@ const {
5959
zeroFill: bindingZeroFill
6060
} = internalBinding('buffer');
6161
const {
62-
arraybuffer_untransferable_private_symbol,
6362
getOwnNonIndexProperties,
6463
propertyFilter: {
6564
ALL_PROPERTIES,
6665
ONLY_ENUMERABLE
6766
},
68-
setHiddenValue,
6967
} = internalBinding('util');
7068
const {
7169
customInspectSymbol,
@@ -83,7 +81,6 @@ const {
8381
} = require('internal/util/inspect');
8482
const { encodings } = internalBinding('string_decoder');
8583

86-
8784
const {
8885
codes: {
8986
ERR_BUFFER_OUT_OF_BOUNDS,
@@ -104,6 +101,7 @@ const {
104101

105102
const {
106103
FastBuffer,
104+
markAsUntransferable,
107105
addBufferPrototypeMethods
108106
} = require('internal/buffer');
109107

@@ -156,7 +154,7 @@ function createUnsafeBuffer(size) {
156154
function createPool() {
157155
poolSize = Buffer.poolSize;
158156
allocPool = createUnsafeBuffer(poolSize).buffer;
159-
setHiddenValue(allocPool, arraybuffer_untransferable_private_symbol, true);
157+
markAsUntransferable(allocPool);
160158
poolOffset = 0;
161159
}
162160
createPool();

‎lib/internal/buffer.js‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ const {
2727
ucs2Write,
2828
utf8Write
2929
} = internalBinding('buffer');
30+
const {
31+
untransferable_object_private_symbol,
32+
setHiddenValue,
33+
} = internalBinding('util');
3034

3135
// Temporary buffers to convert numbers.
3236
const float32Array = new Float32Array(1);
@@ -1007,7 +1011,16 @@ function addBufferPrototypeMethods(proto) {
10071011
proto.utf8Write = utf8Write;
10081012
}
10091013

1014+
// This would better be placed in internal/worker/io.js, but that doesn't work
1015+
// because Buffer needs this and that would introduce a cyclic dependency.
1016+
function markAsUntransferable(obj) {
1017+
if ((typeof obj !== 'object' && typeof obj !== 'function') || obj === null)
1018+
return; // This object is a primitive and therefore already untransferable.
1019+
setHiddenValue(obj, untransferable_object_private_symbol, true);
1020+
}
1021+
10101022
module.exports = {
10111023
FastBuffer,
1012-
addBufferPrototypeMethods
1024+
addBufferPrototypeMethods,
1025+
markAsUntransferable,
10131026
};

‎lib/worker_threads.js‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,18 @@ const {
1212
MessagePort,
1313
MessageChannel,
1414
moveMessagePortToContext,
15-
receiveMessageOnPort
15+
receiveMessageOnPort,
1616
} = require('internal/worker/io');
1717

18+
const {
19+
markAsUntransferable,
20+
} = require('internal/buffer');
21+
1822
module.exports = {
1923
isMainThread,
2024
MessagePort,
2125
MessageChannel,
26+
markAsUntransferable,
2227
moveMessagePortToContext,
2328
receiveMessageOnPort,
2429
resourceLimits,

‎src/env.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,12 +146,12 @@ constexpr size_t kFsStatsBufferLength =
146146
// "node:" prefix to avoid name clashes with third-party code.
147147
#define PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(V) \
148148
V(alpn_buffer_private_symbol, "node:alpnBuffer") \
149-
V(arraybuffer_untransferable_private_symbol, "node:untransferableBuffer") \
150149
V(arrow_message_private_symbol, "node:arrowMessage") \
151150
V(contextify_context_private_symbol, "node:contextify:context") \
152151
V(contextify_global_private_symbol, "node:contextify:global") \
153152
V(decorated_private_symbol, "node:decorated") \
154153
V(napi_wrapper, "node:napi:wrapper") \
154+
V(untransferable_object_private_symbol, "node:untransferableObject") \
155155

156156
// Symbols are per-isolate primitives but Environment proxies them
157157
// for the sake of convenience.

‎src/node_api.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ struct node_napi_env__ : public napi_env__ {
2929
v8::Local<v8::ArrayBuffer> ab) const override {
3030
return ab->SetPrivate(
3131
context(),
32-
node_env()->arraybuffer_untransferable_private_symbol(),
32+
node_env()->untransferable_object_private_symbol(),
3333
v8::True(isolate));
3434
}
3535
};

‎src/node_buffer.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,7 @@ MaybeLocal<Object> New(Environment* env,
419419
Local<ArrayBuffer> ab =
420420
CallbackInfo::CreateTrackedArrayBuffer(env, data, length, callback, hint);
421421
if (ab->SetPrivate(env->context(),
422-
env->arraybuffer_untransferable_private_symbol(),
422+
env->untransferable_object_private_symbol(),
423423
True(env->isolate())).IsNothing()) {
424424
return Local<Object>();
425425
}
@@ -1179,7 +1179,7 @@ void Initialize(Local<Object> target,
11791179
ArrayBuffer::New(env->isolate(), std::move(backing));
11801180
array_buffer->SetPrivate(
11811181
env->context(),
1182-
env->arraybuffer_untransferable_private_symbol(),
1182+
env->untransferable_object_private_symbol(),
11831183
True(env->isolate())).Check();
11841184
CHECK(target
11851185
->Set(env->context(),

‎src/node_messaging.cc‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,21 @@ Maybe<bool> Message::Serialize(Environment* env,
399399
std::vector<Local<ArrayBuffer>> array_buffers;
400400
for (uint32_t i = 0; i < transfer_list_v.length(); ++i) {
401401
Local<Value> entry = transfer_list_v[i];
402-
// Currently, we support ArrayBuffers and MessagePorts.
402+
if (entry->IsObject()) {
403+
// See https://github.com/nodejs/node/pull/30339#issuecomment-552225353
404+
// for details.
405+
bool untransferable;
406+
if (!entry.As<Object>()->HasPrivate(
407+
context,
408+
env->untransferable_object_private_symbol())
409+
.To(&untransferable)) {
410+
return Nothing<bool>();
411+
}
412+
if (untransferable) continue;
413+
}
414+
415+
// Currently, we support ArrayBuffers and BaseObjects for which
416+
// GetTransferMode() does not return kUntransferable.
403417
if (entry->IsArrayBuffer()) {
404418
Local<ArrayBuffer> ab = entry.As<ArrayBuffer>();
405419
// If we cannot render the ArrayBuffer unusable in this Isolate,
@@ -411,16 +425,6 @@ Maybe<bool> Message::Serialize(Environment* env,
411425
// is always going to outlive any Workers it creates, and so will its
412426
// allocator along with it.
413427
if (!ab->IsDetachable()) continue;
414-
// See https://github.com/nodejs/node/pull/30339#issuecomment-552225353
415-
// for details.
416-
bool untransferrable;
417-
if (!ab->HasPrivate(
418-
context,
419-
env->arraybuffer_untransferable_private_symbol())
420-
.To(&untransferrable)) {
421-
return Nothing<bool>();
422-
}
423-
if (untransferrable) continue;
424428
if (std::find(array_buffers.begin(), array_buffers.end(), ab) !=
425429
array_buffers.end()) {
426430
ThrowDataCloneException(

‎test/parallel/test-worker-message-transfer-port-mark-as-untransferable.js‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const { MessageChannel, markAsUntransferable } = require('worker_threads');
5+
6+
{
7+
const ab = new ArrayBuffer(8);
8+
9+
markAsUntransferable(ab);
10+
assert.strictEqual(ab.byteLength, 8);
11+
12+
const { port1, port2 } = new MessageChannel();
13+
port1.postMessage(ab, [ ab ]);
14+
15+
assert.strictEqual(ab.byteLength, 8); // The AB is not detached.
16+
port2.once('message', common.mustCall());
17+
}
18+
19+
{
20+
const channel1 = new MessageChannel();
21+
const channel2 = new MessageChannel();
22+
23+
markAsUntransferable(channel2.port1);
24+
25+
assert.throws(() => {
26+
channel1.port1.postMessage(channel2.port1, [ channel2.port1 ]);
27+
}, /was found in message but not listed in transferList/);
28+
29+
channel2.port1.postMessage('still works, not closed/transferred');
30+
channel2.port2.once('message', common.mustCall());
31+
}
32+
33+
{
34+
for (const value of [0, null, false, true, undefined, [], {}]) {
35+
markAsUntransferable(value); // Has no visible effect.
36+
}
37+
}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL