esm: spawn only one hooks thread by GeoffreyBooth · Pull Request #50752 · nodejs/node · GitHub
Skip to content

esm: spawn only one hooks thread#50752

Closed
GeoffreyBooth wants to merge 1 commit into
nodejs:mainfrom
GeoffreyBooth:fix-spawning-hooks-thread
Closed

esm: spawn only one hooks thread#50752
GeoffreyBooth wants to merge 1 commit into
nodejs:mainfrom
GeoffreyBooth:fix-spawning-hooks-thread

Conversation

@GeoffreyBooth

Copy link
Copy Markdown
Member

Our intent with moving module customization hooks into a separate thread was that one such “hooks thread” would be spawned regardless of however many worker threads the user’s application code spawned. On current main this is not the case; a new hooks thread is created alongside each new worker thread.

This PR aims to fix that, but currently all I have is a test that fails on current main but should pass, once this bug is fixed. I’m opening this as a placeholder for when a fix is ready, and I encourage anyone who wants to take a crack at it to help make this test pass. I’m happy to let others push commits on my branch.

cc @nodejs/loaders @nodejs/workers

@GeoffreyBooth GeoffreyBooth added module Issues and PRs related to the module subsystem. esm Issues and PRs related to the ECMAScript Modules implementation. worker Issues and PRs related to Worker support. loaders Issues and PRs related to ES module loaders labels Nov 16, 2023
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Nov 16, 2023
Comment thread test/es-module/test-esm-loader-threads.mjs Outdated
Comment thread test/fixtures/es-module-loaders/workers-spawned.mjs Outdated
@GeoffreyBooth GeoffreyBooth force-pushed the fix-spawning-hooks-thread branch from 3bad36e to 4480771 Compare January 30, 2024 05:28
@GeoffreyBooth GeoffreyBooth added the help wanted Issues that need assistance from volunteers or PRs that need help to proceed. label Feb 1, 2024
@GeoffreyBooth GeoffreyBooth added loaders-agenda Issues and PRs to discuss during the meetings of the Loaders team and removed loaders-agenda Issues and PRs to discuss during the meetings of the Loaders team labels Feb 26, 2024
@JakobJingleheimer

JakobJingleheimer commented Feb 26, 2024

Copy link
Copy Markdown
Member

@GeoffreyBooth

Copy link
Copy Markdown
Member Author

Fixed by #52706

@GeoffreyBooth GeoffreyBooth removed help wanted Issues that need assistance from volunteers or PRs that need help to proceed. loaders-agenda Issues and PRs to discuss during the meetings of the Loaders team labels May 21, 2024
@GeoffreyBooth GeoffreyBooth deleted the fix-spawning-hooks-thread branch May 21, 2024 02:28
@GeoffreyBooth

Copy link
Copy Markdown
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

esm Issues and PRs related to the ECMAScript Modules implementation. loaders Issues and PRs related to ES module loaders module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. worker Issues and PRs related to Worker support.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants