diff options
author | Anna Henningsen <anna@addaleax.net> | 2019-07-06 22:09:52 +0200 |
---|---|---|
committer | Rich Trott <rtrott@gmail.com> | 2019-07-08 21:03:05 -0700 |
commit | db55c3cfc14775cda810efc58bb8d7c1b7c356c4 (patch) | |
tree | c9f7570cd46dfddace659fae1eabb9edcf65e0c3 | |
parent | 7e50bb3dce11988284c7ca9e2316c086aefac1e1 (diff) | |
download | android-node-v8-db55c3cfc14775cda810efc58bb8d7c1b7c356c4.tar.gz android-node-v8-db55c3cfc14775cda810efc58bb8d7c1b7c356c4.tar.bz2 android-node-v8-db55c3cfc14775cda810efc58bb8d7c1b7c356c4.zip |
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>
-rw-r--r-- | src/node_messaging.cc | 10 | ||||
-rw-r--r-- | test/parallel/test-worker-message-port-multiple-sharedarraybuffers.js | 17 |
2 files changed, 24 insertions, 3 deletions
diff --git a/src/node_messaging.cc b/src/node_messaging.cc index 7a0f2db883..46f06b747e 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -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; diff --git a/test/parallel/test-worker-message-port-multiple-sharedarraybuffers.js b/test/parallel/test-worker-message-port-multiple-sharedarraybuffers.js new file mode 100644 index 0000000000..eb68236e4d --- /dev/null +++ b/test/parallel/test-worker-message-port-multiple-sharedarraybuffers.js @@ -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); |