worker: handle detached MessagePort from a different context

juanarbol · targos · commit 87be790fa925 · 2023-10-23T08:19:46.000+02:00
When `worker.moveMessagePortToContext` is used, the async handle associated with the port, will be triggered more than needed (at least one more time) and with null data. That can be avoided by simply checking that the data is present and the port is not detached. Fixes: #49075 Signed-off-by: Juan José Arboleda <soyjuanarbol@gmail.com> PR-URL: #49150 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
diff --git a/src/node_messaging.cc b/src/node_messaging.cc
@@ -789,6 +789,13 @@ MaybeLocal<Value> MessagePort::ReceiveMessage(Local<Context> context,
 
 void MessagePort::OnMessage(MessageProcessingMode mode) {
   Debug(this, "Running MessagePort::OnMessage()");
+  // Maybe the async handle was triggered empty or more than needed.
+  // The data_ could be freed or, the handle has been/is being closed.
+  // A possible case for this, is transfer the MessagePort to another
+  // context, it will call the constructor and trigger the async handle empty.
+  // Because all data was sent from the preivous context.
+  if (IsDetached()) return;
+
   HandleScope handle_scope(env()->isolate());
   Local<Context> context =
       object(env()->isolate())->GetCreationContext().ToLocalChecked();
diff --git a/test/parallel/test-worker-workerdata-messageport.js b/test/parallel/test-worker-workerdata-messageport.js
@@ -1,11 +1,11 @@
 'use strict';
 
 require('../common');
-const assert = require('assert');
+const assert = require('node:assert');
 
 const {
   Worker, MessageChannel
-} = require('worker_threads');
+} = require('node:worker_threads');
 
 const channel = new MessageChannel();
 const workerData = { mesage: channel.port1 };
@@ -61,3 +61,29 @@ const meowScript = () => 'meow';
              'listed in transferList'
   });
 }
+
+{
+  // Should not crash when MessagePort is transferred to another context.
+  // https://github.com/nodejs/node/issues/49075
+  const channel = new MessageChannel();
+  new Worker(`
+    const { runInContext, createContext } = require('node:vm')
+    const { workerData } = require('worker_threads');
+    const context = createContext(Object.create(null));
+    context.messagePort = workerData.messagePort;
+    runInContext(
+      \`messagePort.postMessage("Meow")\`,
+      context,
+      { displayErrors: true }
+    );
+    `, {
+    eval: true,
+    workerData: { messagePort: channel.port2 },
+    transferList: [channel.port2]
+  });
+  channel.port1.on(
+    'message',
+    (message) =>
+      assert.strictEqual(message, 'Meow')
+  );
+}
