Repository: denoland/deno_core
Pull request #746 (2024-05-21 00:30:48)

fix(core): don't crash when user breaks promise global
Fixes: https://github.com/denoland/deno_core/issues/742
Fixes: https://github.com/denoland/deno_core/issues/743

`pending_mod_evaluation` is set before calling Module::Evaluate, and unset in the promise handlers for the promise returned by evaluate. If setting up those promise handlers fails for some reason, we can end up in a case where evaluation has finished but we haven't unset `pending_mod_evaluation`, which makes the core event loop assume that there is a pending stalled tla message, and it ends up panicking.


Patch:
diff --git a/core/modules/map.rs b/core/modules/map.rs
index 773953b..743b73b 100644
--- a/core/modules/map.rs
+++ b/core/modules/map.rs
@@ -1090,12 +1090,27 @@ impl ModuleMap {
           .then2(tc_scope, on_fulfilled.unwrap(), on_rejected.unwrap())
           .is_none()
       {
-        // The runtime might have been shut down, so we need to synthesize a promise result here
+        // There are two reasons we could be here:
+        // 1. The runtime is shutting down, and JS ops are disabled with termination exceptions.
+        // 2. User code has tampered with the runtime globals in some way that prevents us from
+        //    attaching `on_fulfilled`/`on_rejected` to `promise`.
+        // In these cases we still need to report something back, so synthesize the result from the
+        // promise.
+
+        // Unset pending mod evaluation as the handlers will never run. See debug_assert below.
+        self.pending_mod_evaluation.set(false);
+
         let mut sender = get_sender(evaluation.into());
         match promise.state() {
           PromiseState::Fulfilled => {
-            // Module loaded OK
-            sender.notify(tc_scope);
+            if let Some(exception) = tc_scope.exception() {
+              _ = sender.sender.take().unwrap().send(exception_to_err_result(
+                tc_scope, exception, true, false,
+              ));
+            } else {
+              // Module loaded OK
+              sender.notify(tc_scope);
+            }
           }
           PromiseState::Rejected => {
             // Module was rejected
@@ -1104,6 +1119,11 @@ impl ModuleMap {
             _ = sender.sender.take().unwrap().send(Err(err.into()));
           }
           PromiseState::Pending => {
+            // User code shouldn't be able to both cause the runtime to fail and leave the promise as
+            // pending because the only way to adopt a pending promise is to use `await` and
+            // `await` won't work if you've broken the runtime in such a way that `promise::then`
+            // didn't work.
+            debug_assert!(tc_scope.is_execution_terminating());
             // Module pending, just drop the sender at this point -- we can't do anything with a shut-down runtime.
             drop(sender);
           }


Test patch:
diff --git a/testing/integration/user_breaks_promise_constructor/user_breaks_promise_constructor.out b/testing/integration/user_breaks_promise_constructor/user_breaks_promise_constructor.out
new file mode 100644
index 0000000..9c2a539
--- /dev/null
+++ b/testing/integration/user_breaks_promise_constructor/user_breaks_promise_constructor.out
@@ -0,0 +1,3 @@
+1
+2
+[ERR] Uncaught (in promise) x
diff --git a/testing/integration/user_breaks_promise_constructor/user_breaks_promise_constructor.ts b/testing/integration/user_breaks_promise_constructor/user_breaks_promise_constructor.ts
new file mode 100644
index 0000000..0f84fbc
--- /dev/null
+++ b/testing/integration/user_breaks_promise_constructor/user_breaks_promise_constructor.ts
@@ -0,0 +1,9 @@
+// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
+// https://github.com/denoland/deno_core/issues/743
+console.log("1");
+Object.defineProperty(Promise.prototype, "constructor", {
+  get() {
+    throw "x";
+  },
+});
+console.log("2");
diff --git a/testing/integration/user_breaks_promise_species/user_breaks_promise_species.out b/testing/integration/user_breaks_promise_species/user_breaks_promise_species.out
new file mode 100644
index 0000000..8bc9536
--- /dev/null
+++ b/testing/integration/user_breaks_promise_species/user_breaks_promise_species.out
@@ -0,0 +1,4 @@
+1
+2
+[ERR] TypeError: object.constructor[Symbol.species] is not a constructor
+[ERR]     at Promise.then (<anonymous>)
diff --git a/testing/integration/user_breaks_promise_species/user_breaks_promise_species.ts b/testing/integration/user_breaks_promise_species/user_breaks_promise_species.ts
new file mode 100644
index 0000000..225ac46
--- /dev/null
+++ b/testing/integration/user_breaks_promise_species/user_breaks_promise_species.ts
@@ -0,0 +1,5 @@
+// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
+// https://github.com/denoland/deno_core/issues/742
+console.log("1");
+Object.defineProperty(Promise, Symbol.species, { value: 0 });
+console.log("2");
diff --git a/testing/lib.rs b/testing/lib.rs
index 159c76e..0655022 100644
--- a/testing/lib.rs
+++ b/testing/lib.rs
@@ -69,6 +69,8 @@ integration_test!(
   timer_ref_and_cancel,
   timer_many,
   ts_types,
+  user_breaks_promise_constructor,
+  user_breaks_promise_species,
   worker_spawn,
   worker_terminate,
   worker_terminate_op,