worker: fix nullptr deref after MessagePort deser failure

This would previously always have crashed when deserializing a `MessagePort` fails, because there was always at least one `nullptr` entry in the vector. PR-URL: nodejs#25076 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>
refack · Jan 10, 2019 · 2fdb78c · 2fdb78c
1 parent 8518a3f
commit 2fdb78c
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 1 deletion.
diff --git a/src/node_messaging.cc b/src/node_messaging.cc
@@ -90,7 +90,8 @@ MaybeLocal<Value> Message::Deserialize(Environment* env,
     if (ports[i] == nullptr) {
       for (MessagePort* port : ports) {
         // This will eventually release the MessagePort object itself.
-        port->Close();
+        if (port != nullptr)
+          port->Close();
       }
       return MaybeLocal<Value>();
     }

diff --git a/test/parallel/test-worker-messageport-transfer-terminate.js b/test/parallel/test-worker-messageport-transfer-terminate.js
@@ -0,0 +1,18 @@
+// Flags: --experimental-worker
+'use strict';
+require('../common');
+const { Worker, MessageChannel } = require('worker_threads');
+
+// Check the interaction of calling .terminate() while transferring
+// MessagePort objects; in particular, that it does not crash the process.
+
+for (let i = 0; i < 10; ++i) {
+  const w = new Worker(
+    "require('worker_threads').parentPort.on('message', () => {})",
+    { eval: true });
+  setImmediate(() => {
+    const port = new MessageChannel().port1;
+    w.postMessage({ port }, [ port ]);
+    w.terminate();
+  });
+}