From f100bf81d8bbcea4d88e092b6ae5755f62302ac9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Jos=C3=A9=20Arboleda?= Date: Sun, 13 Aug 2023 14:33:10 -0500 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 --- src/node_messaging.cc | 5 ++++ .../test-worker-workerdata-messageport.js | 30 +++++++++++++++++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/node_messaging.cc b/src/node_messaging.cc index d3d2070d623c80..77e39a0a2625e0 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -795,6 +795,11 @@ void MessagePort::OnMessage(MessageProcessingMode mode) { size_t processing_limit; if (mode == MessageProcessingMode::kNormalOperation) { + // Maybe the async handle was triggered empty or more than needed + // (called twiced on a new context). + if (GetTransferMode() == TransferMode::kDisallowCloneAndTransfer || !data_) + return; + Mutex::ScopedLock(data_->mutex_); processing_limit = std::max(data_->incoming_messages_.size(), static_cast(1000)); diff --git a/test/parallel/test-worker-workerdata-messageport.js b/test/parallel/test-worker-workerdata-messageport.js index 18f05731e8f635..0df48465a071b1 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 }; @@ -59,3 +59,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') + ); +}