buffer: disallow ArrayBuffer transfer on pooled buffer

Author: legendecas

doc/api/worker_threads.md

@@ -256,6 +256,8 @@ 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.
 For example, Node.js marks the `ArrayBuffer`s it uses for its
 [`Buffer` pool][`Buffer.allocUnsafe()`] with this.
+`ArrayBuffer.prototype.transfer()` is disallowed on such array buffer
+instances.
 
 This operation cannot be undone.
 

lib/buffer.js

@@ -143,6 +143,13 @@ const {
   utf8Write,
 } = require('internal/buffer');
 
+const {
+  namespace: {
+    addDeserializeCallback,
+    isBuildingSnapshot,
+  },
+} = require('internal/v8/startup_snapshot');
+
 FastBuffer.prototype.constructor = Buffer;
 Buffer.prototype = FastBuffer.prototype;
 addBufferPrototypeMethods(Buffer.prototype);
@@ -173,6 +180,13 @@ function createPool() {
   poolOffset = 0;
 }
 createPool();
+if (isBuildingSnapshot()) {
+  addDeserializeCallback(() => {
+    // TODO(legendecas): ArrayBuffer.[[ArrayBufferDetachKey]] is not been serialized.
+    // Remove this callback when snapshot serialization supports it.
+    createPool();
+  });
+}
 
 function alignPool() {
   // Ensure aligned slices

lib/internal/buffer.js

@@ -6,6 +6,7 @@ const {
   Float64Array,
   MathFloor,
   Number,
+  Symbol,
   Uint8Array,
 } = primordials;
 
@@ -15,6 +16,8 @@ const {
   ERR_OUT_OF_RANGE,
 } = require('internal/errors').codes;
 const { validateNumber } = require('internal/validators');
+const { isArrayBuffer } = require('util/types');
+
 const {
   asciiSlice,
   base64Slice,
@@ -31,6 +34,7 @@ const {
   ucs2Write,
   utf8WriteStatic,
   createUnsafeArrayBuffer,
+  setDetachKey,
 } = internalBinding('buffer');
 
 const {
@@ -1074,6 +1078,10 @@ function markAsUntransferable(obj) {
   if ((typeof obj !== 'object' && typeof obj !== 'function') || obj === null)
     return;  // This object is a primitive and therefore already untransferable.
   obj[untransferable_object_private_symbol] = true;
+
+  if (isArrayBuffer(obj)) {
+    setDetachKey(obj, Symbol('unique_detach_key_for_untransferable_arraybuffer'));
+  }
 }
 
 // This simply checks if the object is marked as untransferable and doesn't

src/node_buffer.cc

@@ -1354,6 +1354,15 @@ static void Atob(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(error_code);
 }
 
+static void SetDetachKey(const FunctionCallbackInfo<Value>& args) {
+  CHECK_EQ(args.Length(), 2);
+  CHECK(args[0]->IsArrayBuffer());
+
+  Local<ArrayBuffer> ab = args[0].As<ArrayBuffer>();
+  Local<Value> key = args[1];
+  ab->SetDetachKey(key);
+}
+
 namespace {
 
 std::pair<void*, size_t> DecomposeBufferToParts(Local<Value> buffer) {
@@ -1638,6 +1647,8 @@ void Initialize(Local<Object> target,
                 "utf8WriteStatic",
                 SlowWriteString<UTF8>,
                 &fast_write_string_utf8);
+
+  SetMethod(context, target, "setDetachKey", SetDetachKey);
 }
 
 }  // anonymous namespace
@@ -1692,6 +1703,8 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
 
   registry->Register(Atob);
   registry->Register(Btoa);
+
+  registry->Register(SetDetachKey);
 }
 
 }  // namespace Buffer

test/parallel/test-bootstrap-modules.js

@@ -51,6 +51,7 @@ expected.beforePreExec = new Set([
   'NativeModule events',
   'Internal Binding buffer',
   'Internal Binding string_decoder',
+  'NativeModule util/types',
   'NativeModule internal/buffer',
   'NativeModule buffer',
   'Internal Binding messaging',

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

@@ -21,3 +21,9 @@ assert.throws(() => port1.postMessage(a, [ a.buffer ]), {
 // Verify that the pool ArrayBuffer has not actually been transferred:
 assert.strictEqual(a.buffer, b.buffer);
 assert.strictEqual(a.length, length);
+
+// Verify that ArrayBuffer.prototype.transfer() also throws.
+assert.throws(() => a.buffer.transfer(), {
+  name: 'TypeError',
+});
+assert.strictEqual(a.buffer, b.buffer);