From 2014eba782074752e58847c46e6d21d46ef8a8e1 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 1 Jan 2019 21:42:43 +0100
Subject: [PATCH] worker: use engine-provided deleter for `SharedArrayBuffer`s

Store the full information we have on a given `SharedArrayBuffer`,
and use the deleter provided by the JS engine to free the memory
when that is needed.

This fixes memory lifetime management for WASM buffers that are
passed through a `MessageChannel` (e.g. between threads).

PR-URL: https://github.com/nodejs/node/pull/25307
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
---
 src/sharedarraybuffer_metadata.cc             |  16 +++---
 src/sharedarraybuffer_metadata.h              |   5 +-
 test/fixtures/wasm-threads-shared-memory.wasm | Bin 0 -> 36 bytes
 test/fixtures/wasm-threads-shared-memory.wat  |   4 ++
 .../test-worker-message-port-wasm-threads.js  |  46 ++++++++++++++++++
 5 files changed, 62 insertions(+), 9 deletions(-)
 create mode 100644 test/fixtures/wasm-threads-shared-memory.wasm
 create mode 100644 test/fixtures/wasm-threads-shared-memory.wat
 create mode 100644 test/parallel/test-worker-message-port-wasm-threads.js

diff --git a/src/sharedarraybuffer_metadata.cc b/src/sharedarraybuffer_metadata.cc
index 3e760bc50bebe6..671ad6d6820440 100644
--- a/src/sharedarraybuffer_metadata.cc
+++ b/src/sharedarraybuffer_metadata.cc
@@ -89,8 +89,7 @@ SharedArrayBufferMetadata::ForSharedArrayBuffer(
   }
 
   SharedArrayBuffer::Contents contents = source->Externalize();
-  SharedArrayBufferMetadataReference r(new SharedArrayBufferMetadata(
-      contents.Data(), contents.ByteLength()));
+  SharedArrayBufferMetadataReference r(new SharedArrayBufferMetadata(contents));
   if (r->AssignToSharedArrayBuffer(env, context, source).IsNothing())
     return nullptr;
   return r;
@@ -111,17 +110,22 @@ Maybe<bool> SharedArrayBufferMetadata::AssignToSharedArrayBuffer(
                             obj);
 }
 
-SharedArrayBufferMetadata::SharedArrayBufferMetadata(void* data, size_t size)
-  : data(data), size(size) { }
+SharedArrayBufferMetadata::SharedArrayBufferMetadata(
+    const SharedArrayBuffer::Contents& contents)
+  : contents_(contents) { }
 
 SharedArrayBufferMetadata::~SharedArrayBufferMetadata() {
-  free(data);
+  contents_.Deleter()(contents_.Data(),
+                      contents_.ByteLength(),
+                      contents_.DeleterData());
 }
 
 MaybeLocal<SharedArrayBuffer> SharedArrayBufferMetadata::GetSharedArrayBuffer(
     Environment* env, Local<Context> context) {
   Local<SharedArrayBuffer> obj =
-      SharedArrayBuffer::New(env->isolate(), data, size);
+      SharedArrayBuffer::New(env->isolate(),
+                             contents_.Data(),
+                             contents_.ByteLength());
 
   if (AssignToSharedArrayBuffer(env, context, obj).IsNothing())
     return MaybeLocal<SharedArrayBuffer>();
diff --git a/src/sharedarraybuffer_metadata.h b/src/sharedarraybuffer_metadata.h
index 84bfd224fabcf8..8c753a89c11a88 100644
--- a/src/sharedarraybuffer_metadata.h
+++ b/src/sharedarraybuffer_metadata.h
@@ -46,7 +46,7 @@ class SharedArrayBufferMetadata
   SharedArrayBufferMetadata(const SharedArrayBufferMetadata&) = delete;
 
  private:
-  explicit SharedArrayBufferMetadata(void* data, size_t size);
+  explicit SharedArrayBufferMetadata(const v8::SharedArrayBuffer::Contents&);
 
   // Attach a lifetime tracker object with a reference count to `target`.
   v8::Maybe<bool> AssignToSharedArrayBuffer(
@@ -54,8 +54,7 @@ class SharedArrayBufferMetadata
       v8::Local<v8::Context> context,
       v8::Local<v8::SharedArrayBuffer> target);
 
-  void* data = nullptr;
-  size_t size = 0;
+  v8::SharedArrayBuffer::Contents contents_;
 };
 
 }  // namespace worker
diff --git a/test/fixtures/wasm-threads-shared-memory.wasm b/test/fixtures/wasm-threads-shared-memory.wasm
new file mode 100644
index 0000000000000000000000000000000000000000..24b4d691058fd4f8987fb45e5ff5b9069ff1c36d
GIT binary patch
literal 36
rcmZQbEY4+QU|?WnVPs}xV&`IH%T3MAFREl>VBlcMOUzAWVq^dSU<C!X

literal 0
HcmV?d00001

diff --git a/test/fixtures/wasm-threads-shared-memory.wat b/test/fixtures/wasm-threads-shared-memory.wat
new file mode 100644
index 00000000000000..68a2698df6564e
--- /dev/null
+++ b/test/fixtures/wasm-threads-shared-memory.wat
@@ -0,0 +1,4 @@
+(module
+  (memory $mem 1 2 shared)
+  (export "memory" (memory $mem))
+)
\ No newline at end of file
diff --git a/test/parallel/test-worker-message-port-wasm-threads.js b/test/parallel/test-worker-message-port-wasm-threads.js
new file mode 100644
index 00000000000000..6d4f21d728d1b3
--- /dev/null
+++ b/test/parallel/test-worker-message-port-wasm-threads.js
@@ -0,0 +1,46 @@
+// Flags: --experimental-worker --experimental-wasm-threads
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const { MessageChannel, Worker } = require('worker_threads');
+
+// Test that SharedArrayBuffer instances created from WASM are transferrable
+// through MessageChannels (without crashing).
+
+const fixtures = require('../common/fixtures');
+const wasmSource = fixtures.readSync('wasm-threads-shared-memory.wasm');
+const wasmModule = new WebAssembly.Module(wasmSource);
+const instance = new WebAssembly.Instance(wasmModule);
+
+const { buffer } = instance.exports.memory;
+assert(buffer instanceof SharedArrayBuffer);
+
+{
+  const { port1, port2 } = new MessageChannel();
+  port1.postMessage(buffer);
+  port2.once('message', common.mustCall((buffer2) => {
+    // Make sure serialized + deserialized buffer refer to the same memory.
+    const expected = 'Hello, world!';
+    const bytes = Buffer.from(buffer).write(expected);
+    const deserialized = Buffer.from(buffer2).toString('utf8', 0, bytes);
+    assert.deepStrictEqual(deserialized, expected);
+  }));
+}
+
+{
+  // Make sure we can free WASM memory originating from a thread that already
+  // stopped when we exit.
+  const worker = new Worker(`
+  const { parentPort } = require('worker_threads');
+  const wasmSource = new Uint8Array([${wasmSource.join(',')}]);
+  const wasmModule = new WebAssembly.Module(wasmSource);
+  const instance = new WebAssembly.Instance(wasmModule);
+  parentPort.postMessage(instance.exports.memory);
+  `, { eval: true });
+  worker.once('message', common.mustCall(({ buffer }) => {
+    assert(buffer instanceof SharedArrayBuffer);
+    worker.buf = buffer; // Basically just keep the reference to buffer alive.
+  }));
+  worker.once('exit', common.mustCall());
+  worker.postMessage({ wasmModule });
+}