Skip to content

Commit 3bcf86d

Browse files
joyeecheungaduh95
authored andcommitted
esm: use sync loading/resolving on non-loader-hook thread
ESM resolution and loading is now always synchronous from a non-loader-hook thread. If no asynchrnous loader hooks are registered, the resolution/loading is entirely synchronous. If asynchronous loader hooks are registered, these would be synchronous on the non-loader-hook thread, and asynchronous on the loader hook thread. This avoids several races caused by async/sync loading sharing the same cache. In particular, asynchronous loader hooks now works with `require(esm)` - previously it tends to break due to races. In addition, when an asynchronous loader hook returns a promise that never settles, the main thread no longer silently exits with exit code 13, leaving the code below any module loading calls silently ignored without being executed. Instead, it now throws ERR_ASYNC_LOADER_REQUEST_NEVER_SETTLED which can be caught and handled by the main thread. If the module request comes from `import()`, the never-settling promise is now relayed to the result returned by `import()`. Drive-by: when annotating the error about importing undetectable named exports from CommonJS, it now no longer reload the source code of the CommonJS module, and instead reuses format information cached when the module was loaded for linking. PR-URL: #60380 Fixes: #59666 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Jacob Smith <jacob@frende.me>
1 parent cd5c1ad commit 3bcf86d

File tree

21 files changed

+466
-367
lines changed

21 files changed

+466
-367
lines changed

doc/api/errors.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -703,6 +703,13 @@ by the `node:assert` module.
703703
An attempt was made to register something that is not a function as an
704704
`AsyncHooks` callback.
705705

706+
<a id="ERR_ASYNC_LOADER_REQUEST_NEVER_SETTLED"></a>
707+
708+
### `ERR_ASYNC_LOADER_REQUEST_NEVER_SETTLED`
709+
710+
An operation related to module loading is customized by an asynchronous loader
711+
hook that never settled the promise before the loader thread exits.
712+
706713
<a id="ERR_ASYNC_TYPE"></a>
707714

708715
### `ERR_ASYNC_TYPE`

lib/internal/errors.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1137,6 +1137,8 @@ E('ERR_AMBIGUOUS_ARGUMENT', 'The "%s" argument is ambiguous. %s', TypeError);
11371137
E('ERR_ARG_NOT_ITERABLE', '%s must be iterable', TypeError);
11381138
E('ERR_ASSERTION', '%s', Error);
11391139
E('ERR_ASYNC_CALLBACK', '%s must be a function', TypeError);
1140+
E('ERR_ASYNC_LOADER_REQUEST_NEVER_SETTLED',
1141+
'Async loader request never settled', Error);
11401142
E('ERR_ASYNC_TYPE', 'Invalid name for async "type": %s', TypeError);
11411143
E('ERR_BROTLI_INVALID_PARAM', '%s is not a valid Brotli parameter', RangeError);
11421144
E('ERR_BUFFER_OUT_OF_BOUNDS',

lib/internal/modules/esm/hooks.js

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,15 @@ const {
2323
} = globalThis;
2424

2525
const {
26+
ERR_ASYNC_LOADER_REQUEST_NEVER_SETTLED,
2627
ERR_INTERNAL_ASSERTION,
2728
ERR_INVALID_ARG_TYPE,
2829
ERR_INVALID_ARG_VALUE,
2930
ERR_INVALID_RETURN_PROPERTY_VALUE,
3031
ERR_INVALID_RETURN_VALUE,
3132
ERR_LOADER_CHAIN_INCOMPLETE,
32-
ERR_METHOD_NOT_IMPLEMENTED,
3333
ERR_WORKER_UNSERIALIZABLE_ERROR,
3434
} = require('internal/errors').codes;
35-
const { exitCodes: { kUnsettledTopLevelAwait } } = internalBinding('errors');
3635
const { URLParse } = require('internal/url');
3736
const { canParse: URLCanParse } = internalBinding('url');
3837
const { receiveMessageOnPort } = require('worker_threads');
@@ -117,15 +116,16 @@ function defineImportAssertionAlias(context) {
117116
* via `ModuleLoader.#setAsyncLoaderHooks()`.
118117
* @typedef {object} AsyncLoaderHooks
119118
* @property {boolean} allowImportMetaResolve Whether to allow the use of `import.meta.resolve`.
119+
* @property {boolean} isForAsyncLoaderHookWorker Whether the instance is running on the loader hook worker thread.
120120
* @property {(url: string, context: object, defaultLoad: Function) => Promise<LoadResult>} load
121121
* Calling the asynchronous `load` hook asynchronously.
122-
* @property {(url: string, context: object, defaultLoad: Function) => LoadResult} loadSync
122+
* @property {(url: string, context: object, defaultLoad: Function) => LoadResult} [loadSync]
123123
* Calling the asynchronous `load` hook synchronously.
124124
* @property {(originalSpecifier: string, parentURL: string,
125125
* importAttributes: Record<string, string>) => Promise<ResolveResult>} resolve
126126
* Calling the asynchronous `resolve` hook asynchronously.
127127
* @property {(originalSpecifier: string, parentURL: string,
128-
* importAttributes: Record<string, string>) => ResolveResult} resolveSync
128+
* importAttributes: Record<string, string>) => ResolveResult} [resolveSync]
129129
* Calling the asynchronous `resolve` hook synchronously.
130130
* @property {(specifier: string, parentURL: string) => any} register Register asynchronous loader hooks
131131
* @property {() => void} waitForLoaderHookInitialization Force loading of hooks.
@@ -169,6 +169,8 @@ class AsyncLoaderHooksOnLoaderHookWorker {
169169

170170
allowImportMetaResolve = false;
171171

172+
isForAsyncLoaderHookWorker = true;
173+
172174
/**
173175
* Import and register custom/user-defined module loader hook(s).
174176
* @param {string} urlOrSpecifier
@@ -350,10 +352,6 @@ class AsyncLoaderHooksOnLoaderHookWorker {
350352
};
351353
}
352354

353-
resolveSync(_originalSpecifier, _parentURL, _importAttributes) {
354-
throw new ERR_METHOD_NOT_IMPLEMENTED('resolveSync()');
355-
}
356-
357355
/**
358356
* Provide source that is understood by one of Node's translators.
359357
*
@@ -560,7 +558,10 @@ class AsyncLoaderHookWorker {
560558
debug('wait for signal from worker');
561559
AtomicsWait(this.#lock, WORKER_TO_MAIN_THREAD_NOTIFICATION, 0);
562560
const response = this.#worker.receiveMessageSync();
563-
if (response == null || response.message.status === 'exit') { return; }
561+
if (response == null) { return; }
562+
if (response.message.status === 'exit') {
563+
process.exit(response.message.body);
564+
}
564565

565566
// ! This line catches initialization errors in the worker thread.
566567
this.#unwrapMessage(response);
@@ -647,10 +648,13 @@ class AsyncLoaderHookWorker {
647648
this.#workerNotificationLastId = AtomicsLoad(this.#lock, WORKER_TO_MAIN_THREAD_NOTIFICATION);
648649

649650
response = this.#worker.receiveMessageSync();
651+
debug('got sync message from worker', { method, args, response });
650652
} while (response == null);
651-
debug('got sync response from worker', { method, args });
653+
652654
if (response.message.status === 'never-settle') {
653-
process.exit(kUnsettledTopLevelAwait);
655+
const error = new ERR_ASYNC_LOADER_REQUEST_NEVER_SETTLED();
656+
error.details = { method, args };
657+
throw error;
654658
} else if (response.message.status === 'exit') {
655659
process.exit(response.message.body);
656660
}
@@ -819,6 +823,8 @@ class AsyncLoaderHooksProxiedToLoaderHookWorker {
819823

820824
allowImportMetaResolve = true;
821825

826+
isForAsyncLoaderHookWorker = false;
827+
822828
/**
823829
* Instantiate a module loader that uses user-provided custom loader hooks.
824830
*/

lib/internal/modules/esm/initialize_import_meta.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ function createImportMetaResolve(defaultParentURL, loader, allowParentURL) {
3333
}
3434

3535
try {
36-
({ url } = loader.resolveSync(specifier, parentURL));
36+
const request = { specifier, __proto__: null };
37+
({ url } = loader.resolveSync(parentURL, request));
3738
return url;
3839
} catch (error) {
3940
switch (error?.code) {

0 commit comments

Comments
 (0)