module: allow overriding linked requests for a ModuleWrap

[legendecas]

This allows overriding linked requests for a ModuleWrap. The
statusOverride in vm.SourceTextModule could call moduleWrap.link
a second time when statusOverride of linking is set to undefined.

Overriding of linked requests should be no harm but better to be
avoided. However, this will require a follow-up fix on statusOverride
in vm.SourceTextModule.link.

Fixes: #59480

[nodejs-github-bot]

Review requested:

• @nodejs/loaders
• @nodejs/vm

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.89%. Comparing base (3810024) to head (8b1f4eb).
⚠️ Report is 45 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #59527      +/-   ##
==========================================
+ Coverage   89.86%   89.89%   +0.03%     
==========================================
  Files         656      656              
  Lines      193141   193135       -6     
  Branches    37893    37888       -5     
==========================================
+ Hits       173561   173617      +56     
+ Misses      12103    12054      -49     
+ Partials     7477     7464      -13     

Files with missing lines	Coverage Δ
src/module_wrap.cc	75.42% <ø> (+0.28%)	⬆️

... and 30 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[joyeecheung]

Code LGTM though I am a bit confused by the PR description

The statusOverride in vm.SourceTextModule could call moduleWrap.link
a second time when statusOverride of linking is set to undefined.

It's a plain property AFAICT. It seems the more likely cause is that there isn't sufficient guard in the link method to prevent calling the native link method again on a module from the cache that has already called link? Though as long as instantiate() is not called, that seems fine and can be allowed.

[legendecas]

It's a plain property AFAICT. It seems the more likely cause is that there isn't sufficient guard in the link method to prevent calling the native link method again on a module from the cache that has already called link? Though as long as instantiate() is not called, that seems fine and can be allowed.

The statusOverride is set to linking at the begining of the [kLink] method, and reset to undefined at the end of it:

node/lib/internal/vm/module.js

Line 329 in eaf1c15

this.#statusOverride = 'linking';

node/lib/internal/vm/module.js

Line 368 in eaf1c15

this.#statusOverride = undefined;

This means that the status of the SourceTextModule can regress after [kLink], from linking to unlinked, and causes the race condition.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/68779/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/68793/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/68799/