8000 worker: fix broadcast channel SharedArrayBuffer passing · nodejs/node@20ecc82 · GitHub
[go: up one dir, main page]
Skip to content

Commit 20ecc82

Browse files
addaleaxtargos
addaleax
authored and
targos
committed
worker: fix broadcast channel SharedArrayBuffer passing
Make sure that `SharedArrayBuffer`s can be deserialized on multiple receiving ends. As a drive-by, also fix the condition of the internal assertion that should occur if there are transferables passed to multiple destinations. PR-URL: #36501 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 1ec8516 commit 20ecc82

File tree

3 files changed

+18
-9
lines changed

3 files changed

+18
-9
lines changed

‎src/node_messaging.cc‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,7 @@ MaybeLocal<Value> Message::Deserialize(Environment* env,
157157
// Attach all transferred SharedArrayBuffers to their new Isolate.
158158
for (uint32_t i = 0; i < shared_array_buffers_.size(); ++i) {
159159
Local<SharedArrayBuffer> sab =
160-
SharedArrayBuffer::New(env->isolate(),
161-
std::move(shared_array_buffers_[i]));
160+
SharedArrayBuffer::New(env->isolate(), shared_array_buffers_[i]);
162161
shared_array_buffers.push_back(sab);
163162
}
164 8000 163

@@ -1309,7 +1308,7 @@ Maybe<bool> SiblingGroup::Dispatch(
13091308

13101309
// Transferables cannot be used when there is more
13111310
// than a single destination.
1312-
if (size() > 2 && message->transferables().size()) {
1311+
if (size() > 2 && message->has_transferables()) {
13131312
if (error != nullptr)
13141313
*error = "Transferables cannot be used with multiple destinations.";
13151314
return Nothing<bool>();

‎src/node_messaging.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,9 @@ class Message : public MemoryRetainer {
9494
const std::vector<std::unique_ptr<TransferData>>& transferables() const {
9595
return transferables_;
9696
}
97+
bool has_transferables() const {
98+
return !transferables_.empty() || !array_buffers_.empty();
99+
}
97100

98101
void MemoryInfo(MemoryTracker* tracker) const override;
99102

‎test/parallel/test-worker-broadcastchannel.js‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,12 +111,19 @@ assert.throws(() => new BroadcastChannel(), {
111111
{
112112
const bc1 = new BroadcastChannel('channel3');
113113
const bc2 = new BroadcastChannel('channel3');
114-
bc2.postMessage(new SharedArrayBuffer(10));
115-
bc1.addEventListener('message', common.mustCall(({ data }) => {
116-
assert(data instanceof SharedArrayBuffer);
117-
bc1.close();
118-
bc2.close();
119-
}));
114+
const bc3 = new BroadcastChannel('channel3');
115+
bc3.postMessage(new SharedArrayBuffer(10));
116+
let received = 0;
117+
for (const bc of [bc1, bc2]) {
118+
bc.addEventListener('message', common.mustCall(({ data }) => {
119+
assert(data instanceof SharedArrayBuffer);
120+
if (++received === 2) {
121+
bc1.close();
122+
bc2.close();
123+
bc3.close();
124+
}
125+
}));
126+
}
120127
}
121128

122129
{

0 commit comments

Comments
 (0)
0