Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hanging when using MessagePort from ES customization hooks via --import #52846

Closed
elliotgoodrich opened this issue May 5, 2024 · 3 comments · Fixed by #53637
Closed

Hanging when using MessagePort from ES customization hooks via --import #52846

elliotgoodrich opened this issue May 5, 2024 · 3 comments · Fixed by #53637
Labels
loaders Issues and PRs related to ES module loaders

Comments

@elliotgoodrich
Copy link
Contributor

Version

v21.1.0

Platform

Microsoft Windows NT 10.0.22631.0 x64

Subsystem

No response

What steps will reproduce the bug?

Running node --import ./bootstrap.mjs ./index.mjs will hang with the following code.

// hooks.mjs
export async function initialize({ port }) {
  port.postMessage('initialize');
}
// bootstrap.mjs
import { register } from 'node:module';
import { MessageChannel } from 'node:worker_threads';

const { port1, port2 } = new MessageChannel();

port1.on('message', (msg) => {
  console.log(`msg = ${msg}`);
});

register('./hooks.mjs', {
  parentURL: import.meta.url,
  data: { port: port2 },
  transferList: [port2],
});
// index.mjs
console.log('Hello World');

How often does it reproduce? Is there a required condition?

Reproducible 100% of the time.

What is the expected behavior? Why is that the expected behavior?

For node to print

$ node --import ./bootstrap.mjs ./index.mjs
msg = initialize
Hello World

and return normally.

What do you see instead?

node hangs, but prints the expected to the console

$ node --import ./bootstrap.mjs ./index.mjs
msg = initialize
Hello World

Additional information

node returns normally and no longer hangs if

  1. port.close() is added after port.postMessage('initialize');,
  2. or, removing the call to port1.on('message', ...)

This code was taken and simplified from the example in the documentation here https://nodejs.org/api/module.html#initialize.

@Flarna Flarna added the loaders Issues and PRs related to ES module loaders label May 7, 2024
@dygabo
Copy link
Member

dygabo commented May 8, 2024

This is not hanging but still running IMO. It must be happening because port1.on registers an event handler that keeps the process alive. You could implement on your protocol layer some message that unsubscribes for the 'message' event and that will allow the process to end.
for example modify your message handler to get one message and unsubscribe like this:

  port1.on('message', (msg) => {
  console.log(`msg = ${msg}`);
  port1.off('message');
});

the .off call can be based on any other application logic on your apps side.

@nodejs/loaders please correct me if I'm wrong.

EDIT: tried that now, seems wrong. but you could unref the ports used for the comunication from the customization hooks thread:

port1.on('message', (msg) => {
  console.log(`msg = ${msg}`);
});

port1.unref();
port2.unref();

@elliotgoodrich
Copy link
Contributor Author

Thank you @dygabo! unref()ing port1 seems to be sufficient for my initial example - is there a reason to call port2.unref() as well?

I am happy to add a PR to update the example in the node docs (https://nodejs.org/api/module.html#initialize) so others don't encounter this.

@dygabo
Copy link
Member

dygabo commented May 8, 2024

yes, it should be enough to unref port1 to allow the process to stop.

elliotgoodrich added a commit to elliotgoodrich/node that referenced this issue Jun 29, 2024
When running these examples, `node` fails to return as this
`MessagePort` keeps the event loop active in the main thread unless
it is `unref()`ed.

Fixes: nodejs#52846
elliotgoodrich added a commit to elliotgoodrich/node that referenced this issue Jun 29, 2024
When running these examples, `node` fails to return as this
`MessagePort` keeps the event loop active in the main thread unless
it is `unref()`ed.

Fixes: nodejs#52846
aduh95 pushed a commit that referenced this issue Jul 12, 2024
When running these examples, `node` fails to return as this
`MessagePort` keeps the event loop active in the main thread unless
it is `unref()`ed.

Fixes: #52846
PR-URL: #53637
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
aduh95 pushed a commit that referenced this issue Jul 16, 2024
When running these examples, `node` fails to return as this
`MessagePort` keeps the event loop active in the main thread unless
it is `unref()`ed.

Fixes: #52846
PR-URL: #53637
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
marco-ippolito pushed a commit that referenced this issue Aug 19, 2024
When running these examples, `node` fails to return as this
`MessagePort` keeps the event loop active in the main thread unless
it is `unref()`ed.

Fixes: #52846
PR-URL: #53637
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
marco-ippolito pushed a commit that referenced this issue Aug 19, 2024
When running these examples, `node` fails to return as this
`MessagePort` keeps the event loop active in the main thread unless
it is `unref()`ed.

Fixes: #52846
PR-URL: #53637
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
loaders Issues and PRs related to ES module loaders
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants