From 135053486c4bd112ebd7d0b25c94a8dd346472e6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= <biwanczuk@gmail.com>
Date: Wed, 14 Oct 2020 14:04:09 +0200
Subject: fix: top-level-await module execution (#7946)

This commit changes implementation of top-level-await in "deno_core".

Previously promise returned from module evaluation was not awaited,
leading to out-of-order execution of modules that have TLA. It's been
fixed by changing "JsRuntime::mod_evaluate" to be an async function
that resolves when the promise returned from module evaluation also
resolves. When waiting for promise resolution event loop is polled
repeatedly, until there are no more dynamic imports or pending
ops.
---
 cli/ops/worker_host.rs                   | 10 ++++++++--
 cli/tests/integration_tests.rs           | 22 ++++++++++++++++++++++
 cli/tests/tla/a.js                       |  3 +++
 cli/tests/tla/b.js                       |  7 +++++++
 cli/tests/tla/c.js                       |  3 +++
 cli/tests/tla/d.js                       |  6 ++++++
 cli/tests/tla/order.js                   |  1 +
 cli/tests/tla/parent.js                  |  9 +++++++++
 cli/tests/tla2/a.js                      |  5 +++++
 cli/tests/tla2/b.js                      |  5 +++++
 cli/tests/tla3/b.js                      |  7 +++++++
 cli/tests/tla3/timeout_loop.js           | 23 +++++++++++++++++++++++
 cli/tests/top_level_await_circular.js    |  8 ++++++++
 cli/tests/top_level_await_circular.out   |  7 +++++++
 cli/tests/top_level_await_loop.js        | 18 ++++++++++++++++++
 cli/tests/top_level_await_loop.out       |  5 +++++
 cli/tests/top_level_await_order.js       | 21 +++++++++++++++++++++
 cli/tests/top_level_await_order.out      |  2 ++
 cli/tests/top_level_await_unresolved.js  |  1 +
 cli/tests/top_level_await_unresolved.out |  1 +
 cli/worker.rs                            |  2 +-
 21 files changed, 163 insertions(+), 3 deletions(-)
 create mode 100644 cli/tests/tla/a.js
 create mode 100644 cli/tests/tla/b.js
 create mode 100644 cli/tests/tla/c.js
 create mode 100644 cli/tests/tla/d.js
 create mode 100644 cli/tests/tla/order.js
 create mode 100644 cli/tests/tla/parent.js
 create mode 100644 cli/tests/tla2/a.js
 create mode 100644 cli/tests/tla2/b.js
 create mode 100644 cli/tests/tla3/b.js
 create mode 100644 cli/tests/tla3/timeout_loop.js
 create mode 100644 cli/tests/top_level_await_circular.js
 create mode 100644 cli/tests/top_level_await_circular.out
 create mode 100644 cli/tests/top_level_await_loop.js
 create mode 100644 cli/tests/top_level_await_loop.out
 create mode 100644 cli/tests/top_level_await_order.js
 create mode 100644 cli/tests/top_level_await_order.out
 create mode 100644 cli/tests/top_level_await_unresolved.js
 create mode 100644 cli/tests/top_level_await_unresolved.out

(limited to 'cli')

diff --git a/cli/ops/worker_host.rs b/cli/ops/worker_host.rs
index 8ebf8b9e6..ad915f7f7 100644
--- a/cli/ops/worker_host.rs
+++ b/cli/ops/worker_host.rs
@@ -153,9 +153,15 @@ fn run_worker_thread(
       rt.block_on(load_future)
     };
 
-    if let Err(e) = result {
-      let mut sender = worker.internal_channels.sender.clone();
+    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;
+    }
+
+    if let Err(e) = result {
       sender
         .try_send(WorkerEvent::TerminalError(e))
         .expect("Failed to post message to host");
diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs
index 90dc5a4a8..66a83a655 100644
--- a/cli/tests/integration_tests.rs
+++ b/cli/tests/integration_tests.rs
@@ -2421,6 +2421,28 @@ itest!(wasm_unreachable {
   exit_code: 1,
 });
 
+itest!(top_level_await_order {
+  args: "run --allow-read top_level_await_order.js",
+  output: "top_level_await_order.out",
+});
+
+itest!(top_level_await_loop {
+  args: "run --allow-read top_level_await_loop.js",
+  output: "top_level_await_loop.out",
+});
+
+itest!(top_level_await_circular {
+  args: "run --allow-read top_level_await_circular.js",
+  output: "top_level_await_circular.out",
+  exit_code: 1,
+});
+
+itest!(top_level_await_unresolved {
+  args: "run top_level_await_unresolved.js",
+  output: "top_level_await_unresolved.out",
+  exit_code: 1,
+});
+
 itest!(top_level_await {
   args: "run --allow-read top_level_await.js",
   output: "top_level_await.out",
diff --git a/cli/tests/tla/a.js b/cli/tests/tla/a.js
new file mode 100644
index 000000000..c3ef3f7db
--- /dev/null
+++ b/cli/tests/tla/a.js
@@ -0,0 +1,3 @@
+import order from "./order.js";
+
+order.push("b");
diff --git a/cli/tests/tla/b.js b/cli/tests/tla/b.js
new file mode 100644
index 000000000..3271c92d8
--- /dev/null
+++ b/cli/tests/tla/b.js
@@ -0,0 +1,7 @@
+import order from "./order.js";
+
+await new Promise((resolve) => {
+  setTimeout(resolve, 200);
+});
+
+order.push("a");
diff --git a/cli/tests/tla/c.js b/cli/tests/tla/c.js
new file mode 100644
index 000000000..806eb0a8b
--- /dev/null
+++ b/cli/tests/tla/c.js
@@ -0,0 +1,3 @@
+import order from "./order.js";
+
+order.push("c");
diff --git a/cli/tests/tla/d.js b/cli/tests/tla/d.js
new file mode 100644
index 000000000..2b5fd3c45
--- /dev/null
+++ b/cli/tests/tla/d.js
@@ -0,0 +1,6 @@
+import order from "./order.js";
+
+const end = Date.now() + 500;
+while (end < Date.now()) {}
+
+order.push("d");
diff --git a/cli/tests/tla/order.js b/cli/tests/tla/order.js
new file mode 100644
index 000000000..f213a562c
--- /dev/null
+++ b/cli/tests/tla/order.js
@@ -0,0 +1 @@
+export default ["order"];
diff --git a/cli/tests/tla/parent.js b/cli/tests/tla/parent.js
new file mode 100644
index 000000000..1ecc15463
--- /dev/null
+++ b/cli/tests/tla/parent.js
@@ -0,0 +1,9 @@
+import order from "./order.js";
+import "./a.js";
+import "./b.js";
+import "./c.js";
+import "./d.js";
+
+order.push("parent");
+
+export default order;
diff --git a/cli/tests/tla2/a.js b/cli/tests/tla2/a.js
new file mode 100644
index 000000000..d07bcb94d
--- /dev/null
+++ b/cli/tests/tla2/a.js
@@ -0,0 +1,5 @@
+export default class Foo {
+  constructor(message) {
+    this.message = message;
+  }
+}
diff --git a/cli/tests/tla2/b.js b/cli/tests/tla2/b.js
new file mode 100644
index 000000000..68e357c1e
--- /dev/null
+++ b/cli/tests/tla2/b.js
@@ -0,0 +1,5 @@
+export default class Bar {
+  constructor(message) {
+    this.message = message;
+  }
+}
diff --git a/cli/tests/tla3/b.js b/cli/tests/tla3/b.js
new file mode 100644
index 000000000..b74c659e4
--- /dev/null
+++ b/cli/tests/tla3/b.js
@@ -0,0 +1,7 @@
+import { foo } from "./timeout_loop.js";
+import { collection } from "../top_level_await_circular.js";
+
+console.log("collection in b", collection);
+console.log("foo in b", foo);
+
+export const a = "a";
diff --git a/cli/tests/tla3/timeout_loop.js b/cli/tests/tla3/timeout_loop.js
new file mode 100644
index 000000000..860e6cd2a
--- /dev/null
+++ b/cli/tests/tla3/timeout_loop.js
@@ -0,0 +1,23 @@
+export const foo = "foo";
+
+export function delay(ms) {
+  return new Promise((res) =>
+    setTimeout(() => {
+      res();
+    }, ms)
+  );
+}
+
+let i = 0;
+
+async function timeoutLoop() {
+  await delay(1000);
+  console.log("timeout loop", i);
+  i++;
+  if (i > 5) {
+    return;
+  }
+  timeoutLoop();
+}
+
+timeoutLoop();
diff --git a/cli/tests/top_level_await_circular.js b/cli/tests/top_level_await_circular.js
new file mode 100644
index 000000000..ff2964b6a
--- /dev/null
+++ b/cli/tests/top_level_await_circular.js
@@ -0,0 +1,8 @@
+import { foo } from "./tla3/timeout_loop.js";
+
+export const collection = [];
+
+const mod = await import("./tla3/b.js");
+
+console.log("foo in main", foo);
+console.log("mod", mod);
diff --git a/cli/tests/top_level_await_circular.out b/cli/tests/top_level_await_circular.out
new file mode 100644
index 000000000..d47564441
--- /dev/null
+++ b/cli/tests/top_level_await_circular.out
@@ -0,0 +1,7 @@
+timeout loop 0
+timeout loop 1
+timeout loop 2
+timeout loop 3
+timeout loop 4
+timeout loop 5
+error: Dynamically imported module evaluation is still pending but there are no pending ops. This situation is often caused by unresolved promise.
diff --git a/cli/tests/top_level_await_loop.js b/cli/tests/top_level_await_loop.js
new file mode 100644
index 000000000..384f8d0ed
--- /dev/null
+++ b/cli/tests/top_level_await_loop.js
@@ -0,0 +1,18 @@
+const importsDir = Deno.readDirSync(Deno.realPathSync("./tla2"));
+
+const resolvedPaths = [];
+
+for (const { name } of importsDir) {
+  const filePath = Deno.realPathSync(`./tla2/${name}`);
+  resolvedPaths.push(filePath);
+}
+
+resolvedPaths.sort();
+
+for (const filePath of resolvedPaths) {
+  console.log("loading", filePath);
+  const mod = await import(`file://${filePath}`);
+  console.log("loaded", mod);
+}
+
+console.log("all loaded");
diff --git a/cli/tests/top_level_await_loop.out b/cli/tests/top_level_await_loop.out
new file mode 100644
index 000000000..d704e3afd
--- /dev/null
+++ b/cli/tests/top_level_await_loop.out
@@ -0,0 +1,5 @@
+loading [WILDCARD]a.js
+loaded Module { default: [Function: Foo], [Symbol(Symbol.toStringTag)]: "Module" }
+loading [WILDCARD]b.js
+loaded Module { default: [Function: Bar], [Symbol(Symbol.toStringTag)]: "Module" }
+all loaded
diff --git a/cli/tests/top_level_await_order.js b/cli/tests/top_level_await_order.js
new file mode 100644
index 000000000..30659cdfb
--- /dev/null
+++ b/cli/tests/top_level_await_order.js
@@ -0,0 +1,21 @@
+// Ported from Node
+// https://github.com/nodejs/node/blob/54746bb763ebea0dc7e99d88ff4b379bcd680964/test/es-module/test-esm-tla.mjs
+
+const { default: order } = await import("./tla/parent.js");
+
+console.log("order", JSON.stringify(order));
+
+if (
+  !(
+    order[0] === "order" &&
+    order[1] === "b" &&
+    order[2] === "c" &&
+    order[3] === "d" &&
+    order[4] === "a" &&
+    order[5] === "parent"
+  )
+) {
+  throw new Error("TLA wrong order");
+}
+
+console.log("TLA order correct");
diff --git a/cli/tests/top_level_await_order.out b/cli/tests/top_level_await_order.out
new file mode 100644
index 000000000..4cc27858c
--- /dev/null
+++ b/cli/tests/top_level_await_order.out
@@ -0,0 +1,2 @@
+order ["order","b","c","d","a","parent"]
+TLA order correct
diff --git a/cli/tests/top_level_await_unresolved.js b/cli/tests/top_level_await_unresolved.js
new file mode 100644
index 000000000..231a8cd63
--- /dev/null
+++ b/cli/tests/top_level_await_unresolved.js
@@ -0,0 +1 @@
+await new Promise(() => {});
diff --git a/cli/tests/top_level_await_unresolved.out b/cli/tests/top_level_await_unresolved.out
new file mode 100644
index 000000000..77395f5d0
--- /dev/null
+++ b/cli/tests/top_level_await_unresolved.out
@@ -0,0 +1 @@
+error: Module evaluation is still pending but there are no pending ops or dynamic imports. This situation is often caused by unresolved promise.
diff --git a/cli/worker.rs b/cli/worker.rs
index 97e39d20c..d9d9f61c1 100644
--- a/cli/worker.rs
+++ b/cli/worker.rs
@@ -191,7 +191,7 @@ impl Worker {
   ) -> Result<(), AnyError> {
     let id = self.preload_module(module_specifier).await?;
     self.wait_for_inspector_session();
-    self.js_runtime.mod_evaluate(id)
+    self.js_runtime.mod_evaluate(id).await
   }
 
   /// Returns a way to communicate with the Worker from other threads.
-- 
cgit v1.2.3