worker: fix passing multiple SharedArrayBuffers at once

V8 has a handle scope below each `GetSharedArrayBufferId()` call,
so using a `v8::Local` that outlives that handle scope to store
references to `SharedArrayBuffer`s is invalid and may cause accidental
de-duplication of passed `SharedArrayBuffer`s.

Use a persistent handle instead to address this issue.

Fixes: https://github.com/nodejs/node/issues/28559

PR-URL: https://github.com/nodejs/node/pull/28582
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This commit is contained in:
Anna Henningsen 2019-07-06 22:09:52 +02:00 committed by Rich Trott
parent 7e50bb3dce
commit db55c3cfc1
2 changed files with 24 additions and 3 deletions

View File

@ -19,6 +19,7 @@ using v8::Exception;
using v8::Function;
using v8::FunctionCallbackInfo;
using v8::FunctionTemplate;
using v8::Global;
using v8::HandleScope;
using v8::Isolate;
using v8::Just;
@ -241,8 +242,10 @@ class SerializerDelegate : public ValueSerializer::Delegate {
Local<SharedArrayBuffer> shared_array_buffer) override {
uint32_t i;
for (i = 0; i < seen_shared_array_buffers_.size(); ++i) {
if (seen_shared_array_buffers_[i] == shared_array_buffer)
if (PersistentToLocal::Strong(seen_shared_array_buffers_[i]) ==
shared_array_buffer) {
return Just(i);
}
}
auto reference = SharedArrayBufferMetadata::ForSharedArrayBuffer(
@ -252,7 +255,8 @@ class SerializerDelegate : public ValueSerializer::Delegate {
if (!reference) {
return Nothing<uint32_t>();
}
seen_shared_array_buffers_.push_back(shared_array_buffer);
seen_shared_array_buffers_.emplace_back(
Global<SharedArrayBuffer> { isolate, shared_array_buffer });
msg_->AddSharedArrayBuffer(reference);
return Just(i);
}
@ -289,7 +293,7 @@ class SerializerDelegate : public ValueSerializer::Delegate {
Environment* env_;
Local<Context> context_;
Message* msg_;
std::vector<Local<SharedArrayBuffer>> seen_shared_array_buffers_;
std::vector<Global<SharedArrayBuffer>> seen_shared_array_buffers_;
std::vector<MessagePort*> ports_;
friend class worker::Message;

View File

@ -0,0 +1,17 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const { MessageChannel } = require('worker_threads');
// Regression test for https://github.com/nodejs/node/issues/28559
const obj = [
[ new SharedArrayBuffer(0), new SharedArrayBuffer(1) ],
[ new SharedArrayBuffer(2), new SharedArrayBuffer(3) ]
];
const { port1, port2 } = new MessageChannel();
port1.once('message', common.mustCall((message) => {
assert.deepStrictEqual(message, obj);
}));
port2.postMessage(obj);