Skip to content

Commit

Permalink
worker: use engine-provided deleter for SharedArrayBuffers
Browse files Browse the repository at this point in the history
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: #25307
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
  • Loading branch information
addaleax committed Jan 3, 2019
1 parent 7e15f04 commit 1f1a373
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 9 deletions.
16 changes: 10 additions & 6 deletions src/sharedarraybuffer_metadata.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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>();
Expand Down
5 changes: 2 additions & 3 deletions src/sharedarraybuffer_metadata.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,15 @@ 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(
Environment* env,
v8::Local<v8::Context> context,
v8::Local<v8::SharedArrayBuffer> target);

void* data = nullptr;
size_t size = 0;
v8::SharedArrayBuffer::Contents contents_;
};

} // namespace worker
Expand Down
Binary file added test/fixtures/wasm-threads-shared-memory.wasm
Binary file not shown.
4 changes: 4 additions & 0 deletions test/fixtures/wasm-threads-shared-memory.wat
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
(module
(memory $mem 1 2 shared)
(export "memory" (memory $mem))
)
46 changes: 46 additions & 0 deletions test/parallel/test-worker-message-port-wasm-threads.js
Original file line number Diff line number Diff line change
@@ -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 });
}

0 comments on commit 1f1a373

Please sign in to comment.