ESM Hook deadlocks since 21.2.0 a3e09b3

Version

21.2.0

Bisected to a3e09b3fdd3908a6cc82afa319596d3889513a51

Platform

Darwin moxy.lan 23.1.0 Darwin Kernel Version 23.1.0: Mon Oct 9 21:27:24 PDT 2023; root:xnu-10002.41.9~6/RELEASE_ARM64_T6000 arm64

Subsystem

esm hooks

What steps will reproduce the bug?

echo 'console.log("hello")' > x.js
echo '{}' > package.json
npm i tap
node --import=./node_modules/@tapjs/mock/dist/esm/import.mjs --import=./node_modules/@tapjs/processinfo/dist/esm/import.mjs x.js

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

Always reproduces

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

Expect that it loads the two import arguments, and then runs the file.

What do you see instead?

Deadlocks at the Module.register() call in the second import script.

Additional information

Discussion from slack:

isaacs
7 hours ago
@aduh95
Since a3e09b3fdd3908a6cc82afa319596d3889513a51 landed, node-tap hangs whenever I run it. It seems like it's getting stuck at:

      // Sleep until worker responds.
      AtomicsWait(this.#lock, WORKER_TO_MAIN_THREAD_NOTIFICATION, this.#workerNotificationLastId);

I haven't been able to narrow it down to a satisfyingly minimal reproduction, and for extra 😬 it's order-dependent, but this will reproduce it in any project that has the latest tap installed:

node --import=./node_modules/@tapjs/mock/dist/esm/import.mjs --import=./node_modules/@tapjs/processinfo/dist/esm/import.mjs x.js

(where x.js is just console.log('hello'))

5 replies
Jacob Smith
4 hours ago
Oof. These are very difficult to troubleshoot.
I think i'll have time on Wednesday to take a look (edited)
aduh95
3 hours ago
For reference: https://github.com/nodejs/node/commit/a3e09b3fdd3908a6cc82afa319596d3889513a51
🙏
1

isaacs
9 minutes ago
It seems like what's going on is that the @tapjs/mock needs to make a request on its MessageChannel port in order to finish its resolve hook.
Then it loads @tapjs/processinfo/import, which calls Module.register. This triggers a sync request to the worker thread for 'register', which then triggers the mock's resolve hook to resolve the loader.mjs file, but because it's already awaiting a sync message (the register), it'll never come, because that sync message is waiting for another sync message (the resolve).
isaacs
4 minutes ago
My working theory at the moment is that it started at the "run imports sequentially" because prior to that, it wasn't blocking on Module.register() so any blocking async-made-sync actions in the process of the register call weren't a problem.
isaacs
1 minute ago
Also: seems like it's related to the use of the MessageChannel specifically. Just a timeout doesn't trigger it, but the busywait message processing seems to keep the other message channel from proceeding.

2つの --import スクリプトと x.js でデッドロックを再現し、その後、コミット a3e09b3fdd3908a6cc82afa319596d3889513a51 が関係している Module.register() の経路を調査します。順次インポート中の AtomicsWait と MessageChannel の相互作用を追跡します。両方の import 引数が読み込まれ、x.js がハングせずに hello を出力すれば完了です。