Skip to content

Commit 96aeaf2

Browse files
authored
fix(core): keep lazy_loaded_esm sources across concurrent loads (#34353)
Two concurrent dynamic imports that both transitively depend on the same lazy-loaded ESM module (e.g. `node:assert`) could fail with `Unsupported scheme "node"`. Each `import()` spawns its own `RecursiveModuleLoad`, and the per-load `visited`/`get_id` dedup misses across loads — both queue a load future for the lazy specifier. `take_lazy_esm_source` removed the source on first call, so the second future got `None` and fell through to `loader.load()`, which doesn't handle `node:` builtins. This manifests on vite 8 + react-router 7.15 under their CJS bin, where prettier (loaded via `require`) and vite's `chunks/node.js` both import `node:assert`. Return a cheap clone (via `FastString::into_cheap_copy`) instead of removing the entry. The duplicate `ModuleSource`s are deduplicated by `new_module_with_pending`'s existing `get_id` check at register time. Fixes #34307
1 parent 5d41c46 commit 96aeaf2

6 files changed

Lines changed: 97 additions & 10 deletions

File tree

libs/core/modules/map.rs

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2710,21 +2710,31 @@ impl ModuleMap {
27102710
.contains(specifier)
27112711
}
27122712

2713-
/// Try to take a lazy-loaded ESM source by specifier. Returns the source
2714-
/// code if found, removing it from the lazy sources map.
2713+
/// Get a lazy-loaded ESM source by specifier. Returns a cheap clone of the
2714+
/// source code if found, keeping it in the lazy sources map so concurrent
2715+
/// loads of the same lazy specifier from independent `RecursiveModuleLoad`
2716+
/// instances each get their own copy. The duplicate `ModuleSource`s are
2717+
/// deduplicated by `new_module_with_pending`'s `get_id` check at register
2718+
/// time.
27152719
pub(crate) fn take_lazy_esm_source(
27162720
&self,
27172721
specifier: &str,
27182722
) -> Option<ModuleCodeString> {
27192723
let data = self.data.borrow();
2720-
let source = data.lazy_esm_sources.borrow_mut().remove(specifier);
2721-
if source.is_some() {
2722-
data
2723-
.consumed_lazy_specifiers
2724-
.borrow_mut()
2725-
.insert(specifier.to_string());
2726-
}
2727-
source
2724+
let mut sources = data.lazy_esm_sources.borrow_mut();
2725+
let entry = sources.get_mut(specifier)?;
2726+
// `into_cheap_copy` always returns two cheap handles to the same backing
2727+
// storage (Arc or Static), so we can keep one in the map and return the
2728+
// other. The Owned variant gets promoted to Arc in the process.
2729+
let placeholder = ModuleCodeString::from_static("");
2730+
let owned = std::mem::replace(entry, placeholder);
2731+
let (keep, give) = owned.into_cheap_copy();
2732+
*entry = keep;
2733+
data
2734+
.consumed_lazy_specifiers
2735+
.borrow_mut()
2736+
.insert(specifier.to_string());
2737+
Some(give)
27282738
}
27292739

27302740
pub(crate) fn add_lazy_loaded_esm_source(
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// Copyright 2018-2026 the Deno authors. MIT license.
2+
import { value } from "custom:lazy_shared";
3+
export const a = value;
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// Copyright 2018-2026 the Deno authors. MIT license.
2+
import { value } from "custom:lazy_shared";
3+
export const b = value;
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Copyright 2018-2026 the Deno authors. MIT license.
2+
3+
// Two concurrent dynamic imports that both transitively depend on the same
4+
// lazy-loaded ESM module ("custom:lazy_shared"). Each import() spawns its
5+
// own RecursiveModuleLoad, and the per-load `visited` set does not dedupe
6+
// across loads.
7+
const [a, b] = await Promise.all([
8+
import("./lazy_loaded_concurrent_a.js"),
9+
import("./lazy_loaded_concurrent_b.js"),
10+
]);
11+
if (a.a !== "shared") {
12+
throw new Error("expected a.a to be 'shared', got: " + a.a);
13+
}
14+
if (b.b !== "shared") {
15+
throw new Error("expected b.b to be 'shared', got: " + b.b);
16+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
// Copyright 2018-2026 the Deno authors. MIT license.
2+
export const value = "shared";

libs/core/modules/tests.rs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -661,6 +661,59 @@ async fn test_lazy_load_esm_evaluates_pre_instantiated_sibling() {
661661
result.await.unwrap();
662662
}
663663

664+
/// Regression test for https://github.com/denoland/deno/issues/34307
665+
///
666+
/// Two concurrent dynamic `import()` calls each spawn their own
667+
/// `RecursiveModuleLoad`. If both transitively depend on the same
668+
/// `lazy_loaded_esm` module, their per-load `visited`/`get_id` dedup misses
669+
/// (the first hasn't registered the module yet) and both queue load futures
670+
/// for the same lazy specifier. The first wins `take_lazy_esm_source`; the
671+
/// second used to get `None` and fall through to `loader.load()`, which
672+
/// surfaces "Unsupported scheme" for `node:` builtins in the wild (vite 8
673+
/// + react-router 7.15 under a CJS bin).
674+
#[tokio::test]
675+
async fn test_lazy_loaded_esm_concurrent_loads() {
676+
deno_core::extension!(
677+
test_ext,
678+
lazy_loaded_esm = [
679+
dir "modules/testdata",
680+
"custom:lazy_shared" = "lazy_loaded_shared.js",
681+
]
682+
);
683+
684+
let main =
685+
ModuleSpecifier::parse("file:///lazy_loaded_concurrent_main.js").unwrap();
686+
let a =
687+
ModuleSpecifier::parse("file:///lazy_loaded_concurrent_a.js").unwrap();
688+
let b =
689+
ModuleSpecifier::parse("file:///lazy_loaded_concurrent_b.js").unwrap();
690+
let loader = Rc::new(StaticModuleLoader::new([
691+
(
692+
main.clone(),
693+
crate::ascii_str_include!("testdata/lazy_loaded_concurrent_main.js"),
694+
),
695+
(
696+
a,
697+
crate::ascii_str_include!("testdata/lazy_loaded_concurrent_a.js"),
698+
),
699+
(
700+
b,
701+
crate::ascii_str_include!("testdata/lazy_loaded_concurrent_b.js"),
702+
),
703+
]));
704+
705+
let mut runtime = JsRuntime::new(RuntimeOptions {
706+
extensions: vec![test_ext::init()],
707+
module_loader: Some(loader),
708+
..Default::default()
709+
});
710+
711+
let mod_id = runtime.load_main_es_module(&main).await.unwrap();
712+
let result = runtime.mod_evaluate(mod_id);
713+
runtime.run_event_loop(Default::default()).await.unwrap();
714+
result.await.unwrap();
715+
}
716+
664717
#[test]
665718
fn test_lazy_loaded_script() {
666719
deno_core::extension!(

0 commit comments

Comments
 (0)