Skip to content

Commit 2014eba

Browse files
committed
worker: use engine-provided deleter for SharedArrayBuffers
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 <me@gus.host> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
1 parent e14f864 commit 2014eba

5 files changed

Lines changed: 62 additions & 9 deletions

‎src/sharedarraybuffer_metadata.cc‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,7 @@ SharedArrayBufferMetadata::ForSharedArrayBuffer(
8989
}
9090

9191
SharedArrayBuffer::Contents contents = source->Externalize();
92-
SharedArrayBufferMetadataReference r(newSharedArrayBufferMetadata(
93-
contents.Data(), contents.ByteLength()));
92+
SharedArrayBufferMetadataReference r(newSharedArrayBufferMetadata(contents));
9493
if (r->AssignToSharedArrayBuffer(env, context, source).IsNothing())
9594
returnnullptr;
9695
return r;
@@ -111,17 +110,22 @@ Maybe<bool> SharedArrayBufferMetadata::AssignToSharedArrayBuffer(
111110
obj);
112111
}
113112

114-
SharedArrayBufferMetadata::SharedArrayBufferMetadata(void* data, size_t size)
115-
: data(data), size(size) { }
113+
SharedArrayBufferMetadata::SharedArrayBufferMetadata(
114+
const SharedArrayBuffer::Contents& contents)
115+
: contents_(contents) { }
116116

117117
SharedArrayBufferMetadata::~SharedArrayBufferMetadata() {
118-
free(data);
118+
contents_.Deleter()(contents_.Data(),
119+
contents_.ByteLength(),
120+
contents_.DeleterData());
119121
}
120122

121123
MaybeLocal<SharedArrayBuffer> SharedArrayBufferMetadata::GetSharedArrayBuffer(
122124
Environment* env, Local<Context> context) {
123125
Local<SharedArrayBuffer> obj =
124-
SharedArrayBuffer::New(env->isolate(), data, size);
126+
SharedArrayBuffer::New(env->isolate(),
127+
contents_.Data(),
128+
contents_.ByteLength());
125129

126130
if (AssignToSharedArrayBuffer(env, context, obj).IsNothing())
127131
return MaybeLocal<SharedArrayBuffer>();

‎src/sharedarraybuffer_metadata.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,16 +46,15 @@ class SharedArrayBufferMetadata
4646
SharedArrayBufferMetadata(const SharedArrayBufferMetadata&) = delete;
4747

4848
private:
49-
explicitSharedArrayBufferMetadata(void* data, size_t size);
49+
explicitSharedArrayBufferMetadata(const v8::SharedArrayBuffer::Contents&);
5050

5151
// Attach a lifetime tracker object with a reference count to `target`.
5252
v8::Maybe<bool> AssignToSharedArrayBuffer(
5353
Environment* env,
5454
v8::Local<v8::Context> context,
5555
v8::Local<v8::SharedArrayBuffer> target);
5656

57-
void* data = nullptr;
58-
size_t size = 0;
57+
v8::SharedArrayBuffer::Contents contents_;
5958
};
6059

6160
} // namespace worker
36 Bytes
Binary file not shown.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
(module
2+
(memory$mem12 shared)
3+
(export"memory" (memory$mem))
4+
)
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// Flags: --experimental-worker --experimental-wasm-threads
2+
'use strict';
3+
constcommon=require('../common');
4+
constassert=require('assert');
5+
const{ MessageChannel, Worker }=require('worker_threads');
6+
7+
// Test that SharedArrayBuffer instances created from WASM are transferrable
8+
// through MessageChannels (without crashing).
9+
10+
constfixtures=require('../common/fixtures');
11+
constwasmSource=fixtures.readSync('wasm-threads-shared-memory.wasm');
12+
constwasmModule=newWebAssembly.Module(wasmSource);
13+
constinstance=newWebAssembly.Instance(wasmModule);
14+
15+
const{ buffer }=instance.exports.memory;
16+
assert(bufferinstanceofSharedArrayBuffer);
17+
18+
{
19+
const{ port1, port2 }=newMessageChannel();
20+
port1.postMessage(buffer);
21+
port2.once('message',common.mustCall((buffer2)=>{
22+
// Make sure serialized + deserialized buffer refer to the same memory.
23+
constexpected='Hello, world!';
24+
constbytes=Buffer.from(buffer).write(expected);
25+
constdeserialized=Buffer.from(buffer2).toString('utf8',0,bytes);
26+
assert.deepStrictEqual(deserialized,expected);
27+
}));
28+
}
29+
30+
{
31+
// Make sure we can free WASM memory originating from a thread that already
32+
// stopped when we exit.
33+
constworker=newWorker(`
34+
const { parentPort } = require('worker_threads');
35+
const wasmSource = new Uint8Array([${wasmSource.join(',')}]);
36+
const wasmModule = new WebAssembly.Module(wasmSource);
37+
const instance = new WebAssembly.Instance(wasmModule);
38+
parentPort.postMessage(instance.exports.memory);
39+
`,{eval: true});
40+
worker.once('message',common.mustCall(({ buffer })=>{
41+
assert(bufferinstanceofSharedArrayBuffer);
42+
worker.buf=buffer;// Basically just keep the reference to buffer alive.
43+
}));
44+
worker.once('exit',common.mustCall());
45+
worker.postMessage({ wasmModule });
46+
}

0 commit comments

Comments
 (0)