Skip to content

Commit

Permalink
worker: fix broadcast channel SharedArrayBuffer passing
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
addaleax committed Dec 14, 2020
1 parent 68b6c1a commit 67643e1
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 9 deletions.
5 changes: 2 additions & 3 deletions src/node_messaging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,7 @@ MaybeLocal<Value> Message::Deserialize(Environment* env,
// Attach all transferred SharedArrayBuffers to their new Isolate.
for (uint32_t i = 0; i < shared_array_buffers_.size(); ++i) {
Local<SharedArrayBuffer> sab =
SharedArrayBuffer::New(env->isolate(),
std::move(shared_array_buffers_[i]));
SharedArrayBuffer::New(env->isolate(), shared_array_buffers_[i]);
shared_array_buffers.push_back(sab);
}

Expand Down Expand Up @@ -1309,7 +1308,7 @@ Maybe<bool> SiblingGroup::Dispatch(

// Transferables cannot be used when there is more
// than a single destination.
if (size() > 2 && message->transferables().size()) {
if (size() > 2 && message->has_transferables()) {
if (error != nullptr)
*error = "Transferables cannot be used with multiple destinations.";
return Nothing<bool>();
Expand Down
3 changes: 3 additions & 0 deletions src/node_messaging.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ class Message : public MemoryRetainer {
const std::vector<std::unique_ptr<TransferData>>& transferables() const {
return transferables_;
}
bool has_transferables() const {
return !transferables_.empty() || !array_buffers_.empty();
}

void MemoryInfo(MemoryTracker* tracker) const override;

Expand Down
19 changes: 13 additions & 6 deletions test/parallel/test-worker-broadcastchannel.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,19 @@ assert.throws(() => new BroadcastChannel(), {
{
const bc1 = new BroadcastChannel('channel3');
const bc2 = new BroadcastChannel('channel3');
bc2.postMessage(new SharedArrayBuffer(10));
bc1.addEventListener('message', common.mustCall(({ data }) => {
assert(data instanceof SharedArrayBuffer);
bc1.close();
bc2.close();
}));
const bc3 = new BroadcastChannel('channel3');
bc3.postMessage(new SharedArrayBuffer(10));
let received = 0;
for (const bc of [bc1, bc2]) {
bc.addEventListener('message', common.mustCall(({ data }) => {
assert(data instanceof SharedArrayBuffer);
if (++received === 2) {
bc1.close();
bc2.close();
bc3.close();
}
}));
}
}

{
Expand Down

0 comments on commit 67643e1

Please sign in to comment.