src: throw DataCloneError on transfering untransferable objects

doc/api/worker_threads.md

@@ -130,8 +130,11 @@ added:
   - v12.19.0
 -->
 
+* `object` {any} Any arbitrary JavaScript value.
+
 Mark an object as not transferable. If `object` occurs in the transfer list of
-a [`port.postMessage()`][] call, it is ignored.
+a [`port.postMessage()`][] call, an error is thrown. This is a no-op if
+`object` is a primitive value.
 
 In particular, this makes sense for objects that can be cloned, rather than
 transferred, and which are used by other objects on the sending side.
@@ -150,18 +153,47 @@ const typedArray2 = new Float64Array(pooledBuffer);
 markAsUntransferable(pooledBuffer);
 
 const { port1 } = new MessageChannel();
-port1.postMessage(typedArray1, [ typedArray1.buffer ]);
+try {
+  // This will throw an error, because pooledBuffer is not transferable.
+  port1.postMessage(typedArray1, [ typedArray1.buffer ]);
+} catch (error) {
+  // error.name === 'DataCloneError'
+}
 
 // The following line prints the contents of typedArray1 -- it still owns
-// its memory and has been cloned, not transferred. Without
-// `markAsUntransferable()`, this would print an empty Uint8Array.
+// its memory and has not been transferred. Without
+// `markAsUntransferable()`, this would print an empty Uint8Array and the
+// postMessage call would have succeeded.
 // typedArray2 is intact as well.
 console.log(typedArray1);
 console.log(typedArray2);
 ```
 
 There is no equivalent to this API in browsers.
 
+## `worker.isMarkedAsUntransferable(object)`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+* `object` {any} Any JavaScript value.
+* Returns: {boolean}
+
+Check is an object is marked as not transferable with
+[`markAsUntransferable()`][].
+
+```js
+const { markAsUntransferable, isMarkedAsUntransferable } = require('node:worker_threads');
+
+const pooledBuffer = new ArrayBuffer(8);
+markAsUntransferable(pooledBuffer);
+
+isMarkedAsUntransferable(pooledBuffer);  // Returns true.
+```
+
+There is no equivalent to this API in browsers.
+
 ## `worker.moveMessagePortToContext(port, contextifiedSandbox)`
 
 <!-- YAML
@@ -568,6 +600,10 @@ are part of the channel.
 <!-- YAML
 added: v10.5.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/47604
+    description: An error is thrown when an untransferable object is in the
+                 transfer list.
   - version:
       - v15.14.0
       - v14.18.0

lib/internal/buffer.js

@@ -6,6 +6,7 @@ const {
   Float64Array,
   MathFloor,
   Number,
+  ObjectPrototypeHasOwnProperty,
   Uint8Array,
 } = primordials;
 
@@ -1053,6 +1054,18 @@ function markAsUntransferable(obj) {
   obj[untransferable_object_private_symbol] = true;
 }
 
+// This simply checks if the object is marked as untransferable and doesn't
+// check the ability to be transferred.
+function isMarkedAsUntransferable(obj) {
+  if (obj == null)
+    return false;
+  // v8::Object::HasPrivate checks own properties only.
+  if (!ObjectPrototypeHasOwnProperty(obj, untransferable_object_private_symbol)) {
+    return false;
+  }
+  return obj[untransferable_object_private_symbol] === true;
+}
+
 // A toggle used to access the zero fill setting of the array buffer allocator
 // in C++.
 // |zeroFill| can be undefined when running inside an isolate where we
@@ -1079,6 +1092,7 @@ module.exports = {
   FastBuffer,
   addBufferPrototypeMethods,
   markAsUntransferable,
+  isMarkedAsUntransferable,
   createUnsafeBuffer,
   readUInt16BE,
   readUInt32BE,

lib/internal/modules/esm/worker.js

@@ -30,13 +30,21 @@ const {
   WORKER_TO_MAIN_THREAD_NOTIFICATION,
 } = require('internal/modules/esm/shared_constants');
 const { initializeHooks } = require('internal/modules/esm/utils');
-
+const { isMarkedAsUntransferable } = require('internal/buffer');
 
 function transferArrayBuffer(hasError, source) {
   if (hasError || source == null) return;
-  if (isArrayBuffer(source)) return [source];
-  if (isTypedArray(source)) return [TypedArrayPrototypeGetBuffer(source)];
-  if (isDataView(source)) return [DataViewPrototypeGetBuffer(source)];
+  let arrayBuffer;
+  if (isArrayBuffer(source)) {
+    arrayBuffer = source;
+  } else if (isTypedArray(source)) {
+    arrayBuffer = TypedArrayPrototypeGetBuffer(source);
+  } else if (isDataView(source)) {
+    arrayBuffer = DataViewPrototypeGetBuffer(source);
+  }
+  if (arrayBuffer && !isMarkedAsUntransferable(arrayBuffer)) {
+    return [arrayBuffer];
+  }
 }
 
 function wrapMessage(status, body) {

lib/worker_threads.js

@@ -20,13 +20,15 @@ const {
 
 const {
   markAsUntransferable,
+  isMarkedAsUntransferable,
 } = require('internal/buffer');
 
 module.exports = {
   isMainThread,
   MessagePort,
   MessageChannel,
   markAsUntransferable,
+  isMarkedAsUntransferable,
   moveMessagePortToContext,
   receiveMessageOnPort,
   resourceLimits,

src/env_properties.h

@@ -66,7 +66,7 @@
   V(change_string, "change")                                                   \
   V(channel_string, "channel")                                                 \
   V(chunks_sent_since_last_write_string, "chunksSentSinceLastWrite")           \
-  V(clone_unsupported_type_str, "Cannot transfer object of unsupported type.") \
+  V(clone_unsupported_type_str, "Cannot clone object of unsupported type.")    \
   V(code_string, "code")                                                       \
   V(commonjs_string, "commonjs")                                               \
   V(config_string, "config")                                                   \
@@ -301,6 +301,8 @@
   V(time_to_first_header_string, "timeToFirstHeader")                          \
   V(tls_ticket_string, "tlsTicket")                                            \
   V(transfer_string, "transfer")                                               \
+  V(transfer_unsupported_type_str,                                             \
+    "Cannot transfer object of unsupported type.")                             \
   V(ttl_string, "ttl")                                                         \
   V(type_string, "type")                                                       \
   V(uid_string, "uid")                                                         \

src/node_messaging.cc

@@ -459,11 +459,14 @@ Maybe<bool> Message::Serialize(Environment* env,
               .To(&untransferable)) {
         return Nothing<bool>();
       }
-      if (untransferable) continue;
+      if (untransferable) {
+        ThrowDataCloneException(context, env->transfer_unsupported_type_str());
+        return Nothing<bool>();
+      }
     }
 
     // Currently, we support ArrayBuffers and BaseObjects for which
-    // GetTransferMode() does not return kUntransferable.
+    // GetTransferMode() returns kTransferable.
     if (entry->IsArrayBuffer()) {
       Local<ArrayBuffer> ab = entry.As<ArrayBuffer>();
       // If we cannot render the ArrayBuffer unusable in this Isolate,
@@ -474,7 +477,10 @@ Maybe<bool> Message::Serialize(Environment* env,
       // raw data *and* an Isolate with a non-default ArrayBuffer allocator
       // is always going to outlive any Workers it creates, and so will its
       // allocator along with it.
-      if (!ab->IsDetachable()) continue;
+      if (!ab->IsDetachable() || ab->WasDetached()) {
+        ThrowDataCloneException(context, env->transfer_unsupported_type_str());
+        return Nothing<bool>();
+      }
       if (std::find(array_buffers.begin(), array_buffers.end(), ab) !=
           array_buffers.end()) {
         ThrowDataCloneException(
@@ -524,8 +530,8 @@ Maybe<bool> Message::Serialize(Environment* env,
                 entry.As<Object>()->GetConstructorName()));
         return Nothing<bool>();
       }
-      if (host_object && host_object->GetTransferMode() !=
-              BaseObject::TransferMode::kUntransferable) {
+      if (host_object && host_object->GetTransferMode() ==
+                             BaseObject::TransferMode::kTransferable) {
         delegate.AddHostObject(host_object);
         continue;
       }

test/addons/worker-buffer-callback/test.js

@@ -9,7 +9,10 @@ const { buffer } = require(`./build/${common.buildType}/binding`);
 
 const { port1 } = new MessageChannel();
 const origByteLength = buffer.byteLength;
-port1.postMessage(buffer, [buffer.buffer]);
+assert.throws(() => port1.postMessage(buffer, [buffer.buffer]), {
+  code: 25,
+  name: 'DataCloneError',
+});
 
 assert.strictEqual(buffer.byteLength, origByteLength);
 assert.notStrictEqual(buffer.byteLength, 0);

test/node-api/test_worker_buffer_callback/test.js

@@ -9,7 +9,10 @@ const { buffer } = require(`./build/${common.buildType}/binding`);
 
 const { port1 } = new MessageChannel();
 const origByteLength = buffer.byteLength;
-port1.postMessage(buffer, [buffer]);
+assert.throws(() => port1.postMessage(buffer, [buffer]), {
+  code: 25,
+  name: 'DataCloneError',
+});
 
 assert.strictEqual(buffer.byteLength, origByteLength);
 assert.notStrictEqual(buffer.byteLength, 0);

test/parallel/test-buffer-pool-untransferable.js

@@ -13,7 +13,10 @@ assert.strictEqual(a.buffer, b.buffer);
 const length = a.length;
 
 const { port1 } = new MessageChannel();
-port1.postMessage(a, [ a.buffer ]);
+assert.throws(() => port1.postMessage(a, [ a.buffer ]), {
+  code: 25,
+  name: 'DataCloneError',
+});
 
 // Verify that the pool ArrayBuffer has not actually been transferred:
 assert.strictEqual(a.buffer, b.buffer);

test/parallel/test-worker-message-port-arraybuffer.js

@@ -12,6 +12,13 @@ const { MessageChannel } = require('worker_threads');
   typedArray[0] = 0x12345678;
 
   port1.postMessage(typedArray, [ arrayBuffer ]);
+  assert.strictEqual(arrayBuffer.byteLength, 0);
+  // Transferring again should throw a DataCloneError.
+  assert.throws(() => port1.postMessage(typedArray, [ arrayBuffer ]), {
+    code: 25,
+    name: 'DataCloneError',
+  });
+
   port2.on('message', common.mustCall((received) => {
     assert.strictEqual(received[0], 0x12345678);
     port2.close(common.mustCall());

test/parallel/test-worker-message-port-transfer-native.js

@@ -31,7 +31,7 @@ const { internalBinding } = require('internal/test/binding');
     port1.postMessage(nativeObject);
   }, {
     name: 'DataCloneError',
-    message: /Cannot transfer object of unsupported type\.$/
+    message: /Cannot clone object of unsupported type\.$/
   });
   port1.close();
 }

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

@@ -1,37 +1,59 @@
 'use strict';
 const common = require('../common');
 const assert = require('assert');
-const { MessageChannel, markAsUntransferable } = require('worker_threads');
+const { MessageChannel, markAsUntransferable, isMarkedAsUntransferable } = require('worker_threads');
 
 {
   const ab = new ArrayBuffer(8);
 
   markAsUntransferable(ab);
+  assert.ok(isMarkedAsUntransferable(ab));
   assert.strictEqual(ab.byteLength, 8);
 
-  const { port1, port2 } = new MessageChannel();
-  port1.postMessage(ab, [ ab ]);
+  const { port1 } = new MessageChannel();
+  assert.throws(() => port1.postMessage(ab, [ ab ]), {
+    code: 25,
+    name: 'DataCloneError',
+  });
 
   assert.strictEqual(ab.byteLength, 8);  // The AB is not detached.
-  port2.once('message', common.mustCall());
 }
 
 {
   const channel1 = new MessageChannel();
   const channel2 = new MessageChannel();
 
   markAsUntransferable(channel2.port1);
+  assert.ok(isMarkedAsUntransferable(channel2.port1));
 
   assert.throws(() => {
     channel1.port1.postMessage(channel2.port1, [ channel2.port1 ]);
-  }, /was found in message but not listed in transferList/);
+  }, {
+    code: 25,
+    name: 'DataCloneError',
+  });
 
   channel2.port1.postMessage('still works, not closed/transferred');
   channel2.port2.once('message', common.mustCall());
 }
 
 {
-  for (const value of [0, null, false, true, undefined, [], {}]) {
+  for (const value of [0, null, false, true, undefined]) {
     markAsUntransferable(value);  // Has no visible effect.
+    assert.ok(!isMarkedAsUntransferable(value));
   }
+  for (const value of [[], {}]) {
+    markAsUntransferable(value);
+    assert.ok(isMarkedAsUntransferable(value));
+  }
+}
+
+{
+  // Verifies that the mark is not inherited.
+  class Foo {}
+  markAsUntransferable(Foo.prototype);
+  assert.ok(isMarkedAsUntransferable(Foo.prototype));
+
+  const foo = new Foo();
+  assert.ok(!isMarkedAsUntransferable(foo));
 }