mirror of
https://github.com/denoland/deno.git
synced 2024-12-22 15:24:46 -05:00
fix(modules): Immediately resolve follow-up dyn imports to a dyn imported module (#14958)
When a dynamically imported module gets resolved, any code that comes after an await import() to that module will continue running. However, if that is the last code in the evaluation of another dynamically imported module, that second module will not resolve until the next iteration of the event loop, even though it does not depend on the event loop at all. When the event loop is being blocked by a long-running operation, such as a long-running timer, or by an async op that might never end, such as with workers or BroadcastChannels, that will result in the second dynamically imported module not being resolved for a while, or ever. This change fixes this by running the dynamic module loading steps in a loop until no more dynamic modules can be resolved.
This commit is contained in:
parent
18c9a7ad64
commit
38505db391
7 changed files with 51 additions and 11 deletions
|
@ -2733,3 +2733,8 @@ itest!(test_and_bench_are_noops_in_run {
|
|||
args: "run test_and_bench_in_run.js",
|
||||
output_str: Some(""),
|
||||
});
|
||||
|
||||
itest!(followup_dyn_import_resolved {
|
||||
args: "run --unstable --allow-read followup_dyn_import_resolves/main.ts",
|
||||
output: "followup_dyn_import_resolves/main.ts.out",
|
||||
});
|
||||
|
|
14
cli/tests/testdata/followup_dyn_import_resolves/main.ts
vendored
Normal file
14
cli/tests/testdata/followup_dyn_import_resolves/main.ts
vendored
Normal file
|
@ -0,0 +1,14 @@
|
|||
// https://github.com/denoland/deno/issues/14726
|
||||
|
||||
// Any dynamic modules that are only pending on a TLA import should be resolved
|
||||
// in the same event loop iteration as the imported module.
|
||||
|
||||
// Long-running timer so the event loop doesn't have a next iteration for a
|
||||
// while.
|
||||
setTimeout(() => {}, 24 * 60 * 60 * 1000);
|
||||
|
||||
await import("./sub1.ts");
|
||||
|
||||
// If we reach here, the test is passed.
|
||||
console.log("Done.");
|
||||
Deno.exit();
|
3
cli/tests/testdata/followup_dyn_import_resolves/main.ts.out
vendored
Normal file
3
cli/tests/testdata/followup_dyn_import_resolves/main.ts.out
vendored
Normal file
|
@ -0,0 +1,3 @@
|
|||
sub2
|
||||
sub1
|
||||
Done.
|
2
cli/tests/testdata/followup_dyn_import_resolves/sub1.ts
vendored
Normal file
2
cli/tests/testdata/followup_dyn_import_resolves/sub1.ts
vendored
Normal file
|
@ -0,0 +1,2 @@
|
|||
await import("./sub2.ts");
|
||||
console.log("sub1");
|
1
cli/tests/testdata/followup_dyn_import_resolves/sub2.ts
vendored
Normal file
1
cli/tests/testdata/followup_dyn_import_resolves/sub2.ts
vendored
Normal file
|
@ -0,0 +1 @@
|
|||
console.log("sub2");
|
|
@ -1803,15 +1803,11 @@ import "/a.js";
|
|||
)
|
||||
.unwrap();
|
||||
|
||||
// First poll runs `prepare_load` hook.
|
||||
assert!(matches!(runtime.poll_event_loop(cx, false), Poll::Pending));
|
||||
assert_eq!(prepare_load_count.load(Ordering::Relaxed), 1);
|
||||
|
||||
// Second poll actually loads modules into the isolate.
|
||||
assert!(matches!(
|
||||
runtime.poll_event_loop(cx, false),
|
||||
Poll::Ready(Ok(_))
|
||||
));
|
||||
assert_eq!(prepare_load_count.load(Ordering::Relaxed), 1);
|
||||
assert_eq!(resolve_count.load(Ordering::Relaxed), 7);
|
||||
assert_eq!(load_count.load(Ordering::Relaxed), 1);
|
||||
assert!(matches!(
|
||||
|
|
|
@ -886,13 +886,28 @@ impl JsRuntime {
|
|||
|
||||
// Dynamic module loading - ie. modules loaded using "import()"
|
||||
{
|
||||
let poll_imports = self.prepare_dyn_imports(cx)?;
|
||||
assert!(poll_imports.is_ready());
|
||||
// Run in a loop so that dynamic imports that only depend on another
|
||||
// dynamic import can be resolved in this event loop iteration.
|
||||
//
|
||||
// For example, a dynamically imported module like the following can be
|
||||
// immediately resolved after `dependency.ts` is fully evaluated, but it
|
||||
// wouldn't if not for this loop.
|
||||
//
|
||||
// await delay(1000);
|
||||
// await import("./dependency.ts");
|
||||
// console.log("test")
|
||||
//
|
||||
loop {
|
||||
let poll_imports = self.prepare_dyn_imports(cx)?;
|
||||
assert!(poll_imports.is_ready());
|
||||
|
||||
let poll_imports = self.poll_dyn_imports(cx)?;
|
||||
assert!(poll_imports.is_ready());
|
||||
let poll_imports = self.poll_dyn_imports(cx)?;
|
||||
assert!(poll_imports.is_ready());
|
||||
|
||||
self.evaluate_dyn_imports();
|
||||
if !self.evaluate_dyn_imports() {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
self.check_promise_exceptions()?;
|
||||
}
|
||||
|
@ -1505,7 +1520,9 @@ impl JsRuntime {
|
|||
}
|
||||
}
|
||||
|
||||
fn evaluate_dyn_imports(&mut self) {
|
||||
// Returns true if some dynamic import was resolved.
|
||||
fn evaluate_dyn_imports(&mut self) -> bool {
|
||||
let mut resolved_any = false;
|
||||
let state_rc = Self::state(self.v8_isolate());
|
||||
let mut still_pending = vec![];
|
||||
let pending =
|
||||
|
@ -1536,6 +1553,7 @@ impl JsRuntime {
|
|||
};
|
||||
|
||||
if let Some(result) = maybe_result {
|
||||
resolved_any = true;
|
||||
match result {
|
||||
Ok((dyn_import_id, module_id)) => {
|
||||
self.dynamic_import_resolve(dyn_import_id, module_id);
|
||||
|
@ -1547,6 +1565,7 @@ impl JsRuntime {
|
|||
}
|
||||
}
|
||||
state_rc.borrow_mut().pending_dyn_mod_evaluate = still_pending;
|
||||
resolved_any
|
||||
}
|
||||
|
||||
/// Asynchronously load specified module and all of its dependencies.
|
||||
|
|
Loading…
Reference in a new issue