Skip to content

Commit

Permalink
fix(core): module execution with top level await (denoland#7672)
Browse files Browse the repository at this point in the history
This commit fixes implementation of top level await in "deno_core".

Previously promise returned from module execution was ignored causing to execute
modules out-of-order.

With this commit promise returned from module execution is stored on "JsRuntime"
and event loop is polled until the promise resolves.
  • Loading branch information
bartlomieju committed Oct 6, 2020
1 parent 40324ff commit c7c7677
Show file tree
Hide file tree
Showing 10 changed files with 293 additions and 21 deletions.
7 changes: 7 additions & 0 deletions cli/ops/worker_host.rs
Expand Up @@ -155,6 +155,13 @@ fn run_worker_thread(

if let Err(e) = result {
let mut sender = worker.internal_channels.sender.clone();

// If sender is closed it means that worker has already been closed from
// within using "globalThis.close()"
if sender.is_closed() {
return;
}

sender
.try_send(WorkerEvent::TerminalError(e))
.expect("Failed to post message to host");
Expand Down
10 changes: 10 additions & 0 deletions cli/tests/integration_tests.rs
Expand Up @@ -2662,6 +2662,16 @@ itest!(ignore_require {
exit_code: 0,
});

itest!(top_level_await_bug {
args: "run --allow-read top_level_await_bug.js",
output: "top_level_await_bug.out",
});

itest!(top_level_await_bug2 {
args: "run --allow-read top_level_await_bug2.js",
output: "top_level_await_bug2.out",
});

#[test]
fn cafile_env_fetch() {
use deno_core::url::Url;
Expand Down
2 changes: 2 additions & 0 deletions cli/tests/top_level_await_bug.js
@@ -0,0 +1,2 @@
const mod = await import("./top_level_await_bug_nested.js");
console.log(mod);
1 change: 1 addition & 0 deletions cli/tests/top_level_await_bug.out
@@ -0,0 +1 @@
Module { default: 1, [Symbol(Symbol.toStringTag)]: "Module" }
15 changes: 15 additions & 0 deletions cli/tests/top_level_await_bug2.js
@@ -0,0 +1,15 @@
const mod = await import("./top_level_await_bug_nested.js");
console.log(mod);

const sleep = (n) => new Promise((r) => setTimeout(r, n));

await sleep(100);
console.log("slept");

window.addEventListener("load", () => {
console.log("load event");
});

setTimeout(() => {
console.log("timeout");
}, 1000);
4 changes: 4 additions & 0 deletions cli/tests/top_level_await_bug2.out
@@ -0,0 +1,4 @@
Module { default: 1, [Symbol(Symbol.toStringTag)]: "Module" }
slept
load event
timeout
5 changes: 5 additions & 0 deletions cli/tests/top_level_await_bug_nested.js
@@ -0,0 +1,5 @@
const sleep = (n) => new Promise((r) => setTimeout(r, n));

await sleep(100);

export default 1;
4 changes: 2 additions & 2 deletions cli/worker.rs
Expand Up @@ -189,7 +189,7 @@ impl Worker {
) -> Result<(), AnyError> {
let id = self.preload_module(module_specifier).await?;
self.wait_for_inspector_session();
self.isolate.mod_evaluate(id)
self.isolate.mod_evaluate(id).await
}

/// Loads, instantiates and executes provided source code
Expand All @@ -204,7 +204,7 @@ impl Worker {
.load_module(module_specifier, Some(code))
.await?;
self.wait_for_inspector_session();
self.isolate.mod_evaluate(id)
self.isolate.mod_evaluate(id).await
}

/// Returns a way to communicate with the Worker from other threads.
Expand Down
15 changes: 11 additions & 4 deletions core/modules.rs
Expand Up @@ -341,6 +341,13 @@ pub struct ModuleInfo {
pub name: String,
pub handle: v8::Global<v8::Module>,
pub import_specifiers: Vec<ModuleSpecifier>,
// TODO(bartlomieju): there should be "state"
// field that describes if module is already being loaded,
// so concurent dynamic imports don't introduce dead lock
// pub state: LoadState {
// Loading(shared_future),
// Loaded,
// },
}

/// A symbolic module entity.
Expand Down Expand Up @@ -667,7 +674,7 @@ mod tests {
let a_id_fut = runtime.load_module(&spec, None);
let a_id = futures::executor::block_on(a_id_fut).expect("Failed to load");

runtime.mod_evaluate(a_id).unwrap();
futures::executor::block_on(runtime.mod_evaluate(a_id)).unwrap();
let l = loads.lock().unwrap();
assert_eq!(
l.to_vec(),
Expand Down Expand Up @@ -734,7 +741,7 @@ mod tests {
let result = runtime.load_module(&spec, None).await;
assert!(result.is_ok());
let circular1_id = result.unwrap();
runtime.mod_evaluate(circular1_id).unwrap();
runtime.mod_evaluate(circular1_id).await.unwrap();

let l = loads.lock().unwrap();
assert_eq!(
Expand Down Expand Up @@ -811,7 +818,7 @@ mod tests {
println!(">> result {:?}", result);
assert!(result.is_ok());
let redirect1_id = result.unwrap();
runtime.mod_evaluate(redirect1_id).unwrap();
runtime.mod_evaluate(redirect1_id).await.unwrap();
let l = loads.lock().unwrap();
assert_eq!(
l.to_vec(),
Expand Down Expand Up @@ -961,7 +968,7 @@ mod tests {
let main_id =
futures::executor::block_on(main_id_fut).expect("Failed to load");

runtime.mod_evaluate(main_id).unwrap();
futures::executor::block_on(runtime.mod_evaluate(main_id)).unwrap();

let l = loads.lock().unwrap();
assert_eq!(
Expand Down

0 comments on commit c7c7677

Please sign in to comment.