Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 2014eba

Browse filesBrowse 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
Copy full SHA for 2014eba

File tree

Expand file treeCollapse file tree

5 files changed

+62
-9
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+62
-9
lines changed
Open diff view settings
Collapse file

‎src/sharedarraybuffer_metadata.cc‎

Copy file name to clipboardExpand all lines: src/sharedarraybuffer_metadata.cc
+10-6Lines 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(new SharedArrayBufferMetadata(
93-
contents.Data(), contents.ByteLength()));
92+
SharedArrayBufferMetadataReference r(new SharedArrayBufferMetadata(contents));
9493
if (r->AssignToSharedArrayBuffer(env, context, source).IsNothing())
9594
return nullptr;
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>();
Collapse file

‎src/sharedarraybuffer_metadata.h‎

Copy file name to clipboardExpand all lines: src/sharedarraybuffer_metadata.h
+2-3Lines 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-
explicit SharedArrayBufferMetadata(void* data, size_t size);
49+
explicit SharedArrayBufferMetadata(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
Collapse file
36 Bytes
Binary file not shown.
Collapse file
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
(module
2+
(memory $mem 1 2 shared)
3+
(export "memory" (memory $mem))
4+
)
Collapse file
+46Lines 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+
const common = require('../common');
4+
const assert = 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+
const fixtures = require('../common/fixtures');
11+
const wasmSource = fixtures.readSync('wasm-threads-shared-memory.wasm');
12+
const wasmModule = new WebAssembly.Module(wasmSource);
13+
const instance = new WebAssembly.Instance(wasmModule);
14+
15+
const { buffer } = instance.exports.memory;
16+
assert(buffer instanceof SharedArrayBuffer);
17+
18+
{
19+
const { port1, port2 } = new MessageChannel();
20+
port1.postMessage(buffer);
21+
port2.once('message', common.mustCall((buffer2) => {
22+
// Make sure serialized + deserialized buffer refer to the same memory.
23+
const expected = 'Hello, world!';
24+
const bytes = Buffer.from(buffer).write(expected);
25+
const deserialized = 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+
const worker = new Worker(`
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(buffer instanceof SharedArrayBuffer);
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.