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: #28559 PR-URL: #28582 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
nodejs · Jul 20, 2019 · 0e2cbe6 · 0e2cbe6
1 parent 70c3116
commit 0e2cbe6
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 3 deletions.
diff --git 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
@@ -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);