From 87be790fa92522c34b53e50eb5891ed7ec02fdf0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Juan=20Jos=C3=A9?= <soyjuanarbol@gmail.com>
Date: Sun, 22 Oct 2023 23:20:52 +0200
Subject: [PATCH] worker: handle detached `MessagePort` from a different
 context
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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: https://github.com/nodejs/node/issues/49075
Signed-off-by: Juan José Arboleda <soyjuanarbol@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/49150
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
---
 src/node_messaging.cc                         |  7 +++++
 .../test-worker-workerdata-messageport.js     | 30 +++++++++++++++++--
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/src/node_messaging.cc b/src/node_messaging.cc
index 6d2cbf8726c4e4..908e76f399557d 100644
--- 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
index 3e4770d9a12c34..5e73131fd1ab71 100644
--- 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')
+  );
+}