nodejs / nodejs/node

ESM Hook deadlocks since 21.2.0 a3e09b3

Ouverte
#50,948 19 commentaires 1 réaction 0 personnes assignées Voir sur GitHub

Personne n'a encore pris cette issue.

confirmed-bug esm loaders-agenda module
Langage dominant
JavaScript
Étoiles
122k
Forks
37.3k
Merge moyen
4 j 2 h
PR mergées (30 j)
283

Description

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.

Guide de contribution

Ouvrir le guide de contribution

Par où commencer

  1. Lisez l'issue en entier, puis le guide de contribution du projet.
  2. Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
  3. Forkez le dépôt et travaillez sur une branche.
  4. Ouvrez une pull request qui référence le numéro de l'issue.

Piste de recherche

Reproduisez le deadlock avec les deux scripts --import et x.js, puis inspectez le chemin Module.register() mis en cause par le commit a3e09b3fdd3908a6cc82afa319596d3889513a51. Suivez l’interaction avec AtomicsWait et MessageChannel pendant les imports séquentiels. Le travail est terminé lorsque les deux arguments d’import sont chargés et que x.js affiche hello sans se bloquer.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
javascript, node.js
Domaine
backend
Type d'issue
Bug
Difficulté
5/5
Temps estimé
Plus d'une semaine
Activité
À l'abandon
Clarté
Plutôt claire
Accessibilité débutants
28/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.