Skip to content

Commit 5aaeb48

Browse files
authored
fix(ext/node): run register() hooks in worker thread, add --experimental-loader flag (#33906)
Two related features that together bring Node.js module loader hooks much closer to compatibility: ### 1. Worker-thread architecture for `module.register()` Rearchitects `module.register()` to run async hooks in a dedicated Web Worker thread, matching Node.js's "hooks thread" design. Previously, hook modules were loaded on the main thread, causing deadlocks when the hook module's own imports went through the hook infrastructure. **How it works:** - On first `register()` call, a Web Worker is spawned with inline bootstrap code - The hook module is loaded inside the worker (separate event loop, no deadlock) - Resolve/load requests are forwarded from the main thread to the worker via `postMessage` - Sync hooks from `registerHooks()` still run on the main thread (per Node.js spec, sync before async) ### 2. `--experimental-loader` CLI flag Adds `--experimental-loader` (with `--loader` alias) as a hidden CLI flag for Node.js compatibility. Before the main module runs, each loader module is eagerly imported and its resolve/load hooks are registered. ### 3. Fix sync resolve panic (#33901) Adds a `resolve_cache` to `LoaderHookRegistry` that caches resolve results from the async load phase. During V8's synchronous module instantiation callback, the cache is checked first, returning a sync result instead of panicking on an async response. Closes #23201 Fixes #33901
1 parent d9f981f commit 5aaeb48

17 files changed

Lines changed: 593 additions & 168 deletions

File tree

cli/args/flags.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,6 +1001,7 @@ pub struct Flags {
10011001
pub permission_set: Option<String>,
10021002
pub eszip: bool,
10031003
pub node_conditions: Vec<String>,
1004+
pub experimental_loaders: Vec<String>,
10041005
pub preload: Vec<String>,
10051006
pub require: Vec<String>,
10061007
pub tunnel: bool,
@@ -5502,6 +5503,7 @@ fn runtime_misc_args(app: Command) -> Command {
55025503
.arg(enable_testing_features_arg())
55035504
.arg(trace_ops_arg())
55045505
.arg(eszip_arg())
5506+
.arg(experimental_loader_arg())
55055507
.arg(preload_arg())
55065508
.arg(require_arg())
55075509
}
@@ -5710,6 +5712,17 @@ fn reload_arg() -> Arg {
57105712
.help_heading(DEPENDENCY_MANAGEMENT_HEADING)
57115713
}
57125714

5715+
fn experimental_loader_arg() -> Arg {
5716+
Arg::new("experimental-loader")
5717+
.long("experimental-loader")
5718+
.alias("loader")
5719+
.value_name("MODULE")
5720+
.action(ArgAction::Append)
5721+
.help("Use the specified module as a custom loader (Node.js compat)")
5722+
.hide(true)
5723+
.value_hint(ValueHint::FilePath)
5724+
}
5725+
57135726
fn preload_arg() -> Arg {
57145727
Arg::new("preload")
57155728
.long("preload")
@@ -8270,6 +8283,7 @@ fn runtime_args_parse(
82708283
env_file_arg_parse(flags, matches);
82718284
trace_ops_parse(flags, matches);
82728285
eszip_arg_parse(flags, matches);
8286+
experimental_loader_arg_parse(flags, matches);
82738287
preload_arg_parse(flags, matches);
82748288
require_arg_parse(flags, matches);
82758289
Ok(())
@@ -8320,6 +8334,12 @@ fn reload_arg_parse(
83208334
Ok(())
83218335
}
83228336

8337+
fn experimental_loader_arg_parse(flags: &mut Flags, matches: &mut ArgMatches) {
8338+
if let Some(loaders) = matches.remove_many::<String>("experimental-loader") {
8339+
flags.experimental_loaders = loaders.collect();
8340+
}
8341+
}
8342+
83238343
fn preload_arg_parse(flags: &mut Flags, matches: &mut ArgMatches) {
83248344
if let Some(preload) = matches.remove_many::<String>("preload") {
83258345
flags.preload = preload.collect();

cli/args/mod.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -685,6 +685,19 @@ impl CliOptions {
685685
.filter_map(|p| canonicalize_path(&p).ok())
686686
}
687687

688+
pub fn experimental_loaders(&self) -> Result<Vec<ModuleSpecifier>, AnyError> {
689+
if self.flags.experimental_loaders.is_empty() {
690+
return Ok(vec![]);
691+
}
692+
693+
let mut modules = Vec::with_capacity(self.flags.experimental_loaders.len());
694+
for loader_specifier in self.flags.experimental_loaders.iter() {
695+
modules.push(resolve_url_or_path(loader_specifier, self.initial_cwd())?);
696+
}
697+
698+
Ok(modules)
699+
}
700+
688701
pub fn preload_modules(&self) -> Result<Vec<ModuleSpecifier>, AnyError> {
689702
if self.flags.preload.is_empty() {
690703
return Ok(vec![]);

cli/factory.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1275,6 +1275,7 @@ impl CliFactory {
12751275
Ok(CliMainWorkerOptions {
12761276
needs_test_modules: cli_options.sub_command().needs_test(),
12771277
create_hmr_runner,
1278+
experimental_loaders: cli_options.experimental_loaders()?,
12781279
maybe_coverage_dir,
12791280
maybe_cpu_prof_config,
12801281
default_npm_caching_strategy: cli_options.default_npm_caching_strategy(),

cli/lib/worker.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,12 +587,14 @@ impl<TSys: DenoLibSys> LibMainWorkerFactory<TSys> {
587587
mode: WorkerExecutionMode,
588588
permissions: PermissionsContainer,
589589
main_module: Url,
590+
experimental_loaders: Vec<Url>,
590591
preload_modules: Vec<Url>,
591592
require_modules: Vec<Url>,
592593
) -> Result<LibMainWorker, CoreError> {
593594
self.create_custom_worker(
594595
mode,
595596
main_module,
597+
experimental_loaders,
596598
preload_modules,
597599
require_modules,
598600
permissions,
@@ -607,6 +609,7 @@ impl<TSys: DenoLibSys> LibMainWorkerFactory<TSys> {
607609
&self,
608610
mode: WorkerExecutionMode,
609611
main_module: Url,
612+
experimental_loaders: Vec<Url>,
610613
preload_modules: Vec<Url>,
611614
require_modules: Vec<Url>,
612615
permissions: PermissionsContainer,
@@ -744,6 +747,7 @@ impl<TSys: DenoLibSys> LibMainWorkerFactory<TSys> {
744747

745748
Ok(LibMainWorker {
746749
main_module,
750+
experimental_loaders,
747751
preload_modules,
748752
require_modules,
749753
worker,
@@ -835,6 +839,7 @@ impl<TSys: DenoLibSys> LibMainWorkerFactory<TSys> {
835839

836840
pub struct LibMainWorker {
837841
main_module: Url,
842+
experimental_loaders: Vec<Url>,
838843
preload_modules: Vec<Url>,
839844
require_modules: Vec<Url>,
840845
worker: MainWorker,
@@ -927,6 +932,33 @@ impl LibMainWorker {
927932
Ok(())
928933
}
929934

935+
pub async fn execute_experimental_loaders(
936+
&mut self,
937+
) -> Result<(), CoreError> {
938+
if self.experimental_loaders.is_empty() {
939+
return Ok(());
940+
}
941+
// Build a JS array of loader URLs using JSON serialization
942+
// for robust escaping of special characters in URLs.
943+
let urls_json: Vec<String> = self
944+
.experimental_loaders
945+
.iter()
946+
.map(|u| serde_json::to_string(u.as_str()).unwrap())
947+
.collect();
948+
let script = format!(
949+
r#"(async () => {{
950+
const {{ _registerCliLoaders }} = await import("node:module");
951+
await _registerCliLoaders([{}]);
952+
}})()"#,
953+
urls_json.join(",")
954+
);
955+
self
956+
.worker
957+
.execute_script("experimental-loader-setup", script.into())?;
958+
self.worker.run_event_loop(false).await?;
959+
Ok(())
960+
}
961+
930962
pub async fn execute_preload_modules(&mut self) -> Result<(), CoreError> {
931963
for preload_module_url in self.preload_modules.iter() {
932964
let id = self.worker.preload_side_module(preload_module_url).await?;
@@ -946,6 +978,9 @@ impl LibMainWorker {
946978
pub async fn run(&mut self) -> Result<i32, CoreError> {
947979
log::debug!("main_module {}", self.main_module);
948980

981+
// Register experimental loader hooks before anything else
982+
self.execute_experimental_loaders().await?;
983+
949984
// Run preload modules first if they were defined
950985
self.execute_preload_modules().await?;
951986

cli/module_loader.rs

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,6 +1019,23 @@ impl<TGraphContainer: ModuleGraphContainer> ModuleLoader
10191019
if self.0.hook_registry.resolve_active.get()
10201020
&& !is_already_resolved_specifier(specifier)
10211021
{
1022+
// Check if this specifier was already resolved by hooks during
1023+
// the load phase. This cache lookup is critical for V8's module
1024+
// instantiation callback (module_resolve_callback) which calls
1025+
// resolve synchronously -- we cannot return Async there.
1026+
let cache_key = format!("{specifier}\x00{referrer}");
1027+
if let Some(cached) = self
1028+
.0
1029+
.hook_registry
1030+
.resolve_cache
1031+
.borrow_mut()
1032+
.remove(&cache_key)
1033+
{
1034+
return deno_core::ModuleResolveResponse::Sync(
1035+
ModuleSpecifier::parse(&cached).map_err(JsErrorBox::from_err),
1036+
);
1037+
}
1038+
10221039
let receiver = self
10231040
.0
10241041
.hook_registry
@@ -1039,6 +1056,12 @@ impl<TGraphContainer: ModuleGraphContainer> ModuleLoader
10391056
Ok(Some(url)) => {
10401057
let parsed =
10411058
ModuleSpecifier::parse(&url).map_err(JsErrorBox::from_err)?;
1059+
// Cache the resolve result so instantiation can look it up
1060+
// synchronously later.
1061+
inner.hook_registry.resolve_cache.borrow_mut().insert(
1062+
format!("{specifier}\x00{referrer}"),
1063+
parsed.to_string(),
1064+
);
10421065
// Track that this specifier was hook-intercepted (virtual module)
10431066
// so prepare_load knows to skip graph building for it.
10441067
inner
@@ -1049,8 +1072,17 @@ impl<TGraphContainer: ModuleGraphContainer> ModuleLoader
10491072
Ok(parsed)
10501073
}
10511074
Ok(None) => {
1052-
// Fallthrough: hooks didn't intercept, use default
1053-
inner.inner_resolve(&specifier, &referrer, kind, false)
1075+
// Fallthrough: hooks didn't intercept, use default.
1076+
// Cache the result so instantiation doesn't go async.
1077+
let result =
1078+
inner.inner_resolve(&specifier, &referrer, kind, false);
1079+
if let Ok(ref resolved) = result {
1080+
inner.hook_registry.resolve_cache.borrow_mut().insert(
1081+
format!("{specifier}\x00{referrer}"),
1082+
resolved.to_string(),
1083+
);
1084+
}
1085+
result
10541086
}
10551087
Err(err) => Err(JsErrorBox::generic(err)),
10561088
}

cli/rt/run.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1397,6 +1397,7 @@ pub async fn run(
13971397
WorkerExecutionMode::Run,
13981398
permissions,
13991399
main_module,
1400+
vec![], // no experimental loaders in standalone binaries
14001401
preload_modules,
14011402
require_modules,
14021403
)?;

cli/worker.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ pub type CreateHmrRunnerCb = Box<dyn Fn() -> HmrRunnerState + Send + Sync>;
4343

4444
pub struct CliMainWorkerOptions {
4545
pub create_hmr_runner: Option<CreateHmrRunnerCb>,
46+
pub experimental_loaders: Vec<ModuleSpecifier>,
4647
pub maybe_coverage_dir: Option<PathBuf>,
4748
pub maybe_cpu_prof_config: Option<CpuProfilerConfig>,
4849
pub default_npm_caching_strategy: NpmCachingStrategy,
@@ -131,6 +132,9 @@ impl CliMainWorker {
131132

132133
log::debug!("main_module {}", self.worker.main_module());
133134

135+
// Register experimental loader hooks before anything else
136+
self.worker.execute_experimental_loaders().await?;
137+
134138
// Run preload modules first if they were defined
135139
self.worker.execute_preload_modules().await?;
136140
self.execute_main_module().await?;
@@ -358,6 +362,7 @@ pub enum CreateCustomWorkerError {
358362
}
359363

360364
pub struct CliMainWorkerFactory {
365+
experimental_loaders: Vec<ModuleSpecifier>,
361366
lib_main_worker_factory: LibMainWorkerFactory<CliSys>,
362367
maybe_lockfile: Option<Arc<CliLockfile>>,
363368
npm_installer: Option<Arc<CliNpmInstaller>>,
@@ -382,6 +387,7 @@ impl CliMainWorkerFactory {
382387
root_permissions: PermissionsContainer,
383388
) -> Self {
384389
Self {
390+
experimental_loaders: options.experimental_loaders,
385391
lib_main_worker_factory,
386392
maybe_lockfile,
387393
npm_installer,
@@ -505,6 +511,7 @@ impl CliMainWorkerFactory {
505511
let mut worker = self.lib_main_worker_factory.create_custom_worker(
506512
mode,
507513
main_module,
514+
self.experimental_loaders.clone(),
508515
preload_modules,
509516
require_modules,
510517
permissions,

ext/node/ops/module_hooks.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,12 @@ pub struct LoaderHookRegistry {
4949
/// don't exist on disk.
5050
pub hook_intercepted_specifiers: Rc<RefCell<HashSet<String>>>,
5151

52+
/// Cache of resolve results (key: "specifier\0referrer" → resolved URL).
53+
/// Populated during the async load phase so that V8's synchronous module
54+
/// instantiation callback can look up previously-resolved specifiers
55+
/// without going through the async hook bridge.
56+
pub resolve_cache: Rc<RefCell<HashMap<String, String>>>,
57+
5258
pending_resolves: Rc<RefCell<VecDeque<PendingResolve>>>,
5359
resolve_waker: Rc<RefCell<Option<Waker>>>,
5460
resolve_senders: Rc<RefCell<HashMap<u32, ResolveSender>>>,

0 commit comments

Comments
 (0)