Skip to content

Commit e7dbdd3

Browse files
juanarbolalexfernandez
authored andcommitted
worker: handle detached MessagePort from a different context
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: nodejs#49075 Signed-off-by: Juan José Arboleda <soyjuanarbol@gmail.com> PR-URL: nodejs#49150 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
1 parent 2f8bbda commit e7dbdd3

File tree

2 files changed

+35
-2
lines changed

2 files changed

+35
-2
lines changed

src/node_messaging.cc

+7
Original file line numberDiff line numberDiff line change
@@ -789,6 +789,13 @@ MaybeLocal<Value> MessagePort::ReceiveMessage(Local<Context> context,
789789

790790
void MessagePort::OnMessage(MessageProcessingMode mode) {
791791
Debug(this, "Running MessagePort::OnMessage()");
792+
// Maybe the async handle was triggered empty or more than needed.
793+
// The data_ could be freed or, the handle has been/is being closed.
794+
// A possible case for this, is transfer the MessagePort to another
795+
// context, it will call the constructor and trigger the async handle empty.
796+
// Because all data was sent from the preivous context.
797+
if (IsDetached()) return;
798+
792799
HandleScope handle_scope(env()->isolate());
793800
Local<Context> context =
794801
object(env()->isolate())->GetCreationContext().ToLocalChecked();

test/parallel/test-worker-workerdata-messageport.js

+28-2
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
'use strict';
22

33
require('../common');
4-
const assert = require('assert');
4+
const assert = require('node:assert');
55

66
const {
77
Worker, MessageChannel
8-
} = require('worker_threads');
8+
} = require('node:worker_threads');
99

1010
const channel = new MessageChannel();
1111
const workerData = { mesage: channel.port1 };
@@ -61,3 +61,29 @@ const meowScript = () => 'meow';
6161
'listed in transferList'
6262
});
6363
}
64+
65+
{
66+
// Should not crash when MessagePort is transferred to another context.
67+
// https://github.com/nodejs/node/issues/49075
68+
const channel = new MessageChannel();
69+
new Worker(`
70+
const { runInContext, createContext } = require('node:vm')
71+
const { workerData } = require('worker_threads');
72+
const context = createContext(Object.create(null));
73+
context.messagePort = workerData.messagePort;
74+
runInContext(
75+
\`messagePort.postMessage("Meow")\`,
76+
context,
77+
{ displayErrors: true }
78+
);
79+
`, {
80+
eval: true,
81+
workerData: { messagePort: channel.port2 },
82+
transferList: [channel.port2]
83+
});
84+
channel.port1.on(
85+
'message',
86+
(message) =>
87+
assert.strictEqual(message, 'Meow')
88+
);
89+
}

0 commit comments

Comments
 (0)