diff --git a/core/bindings.rs b/core/bindings.rs index 49dc62c0ee..a88e54af70 100644 --- a/core/bindings.rs +++ b/core/bindings.rs @@ -292,20 +292,12 @@ pub extern "C" fn promise_reject_callback(message: v8::PromiseRejectMessage) { // a) asynchronous, and b) always terminates. if let Some(js_promise_reject_cb) = state.js_promise_reject_cb.clone() { let js_uncaught_exception_cb = state.js_uncaught_exception_cb.clone(); + drop(state); // Drop borrow, callbacks can call back into runtime. let tc_scope = &mut v8::TryCatch::new(scope); let undefined: v8::Local = v8::undefined(tc_scope).into(); let type_ = v8::Integer::new(tc_scope, message.get_event() as i32); let promise = message.get_promise(); - if let Some(pending_mod_evaluate) = state.pending_mod_evaluate.as_mut() { - if !pending_mod_evaluate.has_evaluated { - let promise_global = v8::Global::new(tc_scope, promise); - pending_mod_evaluate - .handled_promise_rejections - .push(promise_global); - } - } - drop(state); // Drop borrow, callbacks can call back into runtime. let reason = match message.get_event() { PromiseRejectWithNoHandler diff --git a/core/runtime.rs b/core/runtime.rs index 64e7f635c0..64985939b1 100644 --- a/core/runtime.rs +++ b/core/runtime.rs @@ -98,9 +98,7 @@ pub(crate) struct DynImportModEvaluate { } pub(crate) struct ModEvaluate { - pub(crate) promise: Option>, - pub(crate) has_evaluated: bool, - pub(crate) handled_promise_rejections: Vec>, + promise: v8::Global, sender: oneshot::Sender>, } @@ -1290,26 +1288,7 @@ impl JsRuntime { // For more details see: // https://github.com/denoland/deno/issues/4908 // https://v8.dev/features/top-level-await#module-execution-order - { - let mut state = state_rc.borrow_mut(); - assert!( - state.pending_mod_evaluate.is_none(), - "There is already pending top level module evaluation" - ); - state.pending_mod_evaluate = Some(ModEvaluate { - promise: None, - has_evaluated: false, - handled_promise_rejections: vec![], - sender, - }); - } - let maybe_value = module.evaluate(tc_scope); - { - let mut state = state_rc.borrow_mut(); - let pending_mod_evaluate = state.pending_mod_evaluate.as_mut().unwrap(); - pending_mod_evaluate.has_evaluated = true; - } // Update status after evaluating. status = module.get_status(); @@ -1318,12 +1297,7 @@ impl JsRuntime { state_rc.borrow_mut().explicit_terminate_exception.take(); if let Some(exception) = explicit_terminate_exception { let exception = v8::Local::new(tc_scope, exception); - let pending_mod_evaluate = { - let mut state = state_rc.borrow_mut(); - state.pending_mod_evaluate.take().unwrap() - }; - pending_mod_evaluate - .sender + sender .send(exception_to_err_result(tc_scope, exception, false)) .expect("Failed to send module evaluation error."); } else if let Some(value) = maybe_value { @@ -1337,15 +1311,18 @@ impl JsRuntime { let mut state = state_rc.borrow_mut(); state.pending_promise_exceptions.remove(&promise_global); let promise_global = v8::Global::new(tc_scope, promise); - state.pending_mod_evaluate.as_mut().unwrap().promise = - Some(promise_global); + assert!( + state.pending_mod_evaluate.is_none(), + "There is already pending top level module evaluation" + ); + + state.pending_mod_evaluate = Some(ModEvaluate { + promise: promise_global, + sender, + }); tc_scope.perform_microtask_checkpoint(); } else if tc_scope.has_terminated() || tc_scope.is_execution_terminating() { - let pending_mod_evaluate = { - let mut state = state_rc.borrow_mut(); - state.pending_mod_evaluate.take().unwrap() - }; - pending_mod_evaluate.sender.send(Err( + sender.send(Err( generic_error("Cannot evaluate module, because JavaScript execution has been terminated.") )).expect("Failed to send module evaluation error."); } else { @@ -1553,11 +1530,10 @@ impl JsRuntime { return; } - let mut module_evaluation = maybe_module_evaluation.unwrap(); + let module_evaluation = maybe_module_evaluation.unwrap(); let scope = &mut self.handle_scope(); - let promise_global = module_evaluation.promise.clone().unwrap(); - let promise = promise_global.open(scope); + let promise = module_evaluation.promise.open(scope); let promise_state = promise.state(); match promise_state { @@ -1570,24 +1546,14 @@ impl JsRuntime { scope.perform_microtask_checkpoint(); // Receiver end might have been already dropped, ignore the result let _ = module_evaluation.sender.send(Ok(())); - module_evaluation.handled_promise_rejections.clear(); } v8::PromiseState::Rejected => { let exception = promise.result(scope); scope.perform_microtask_checkpoint(); - // Receiver end might have been already dropped, ignore the result - if module_evaluation - .handled_promise_rejections - .contains(&promise_global) - { - let _ = module_evaluation.sender.send(Ok(())); - module_evaluation.handled_promise_rejections.clear(); - } else { - let _ = module_evaluation - .sender - .send(exception_to_err_result(scope, exception, false)); - } + let _ = module_evaluation + .sender + .send(exception_to_err_result(scope, exception, false)); } } } @@ -3288,95 +3254,6 @@ assertEquals(1, notify_return_value); assert_eq!(2, UNCAUGHT_EXCEPTION.load(Ordering::Relaxed)); } - #[tokio::test] - async fn test_set_promise_reject_callback_top_level_await() { - static PROMISE_REJECT: AtomicUsize = AtomicUsize::new(0); - static UNCAUGHT_EXCEPTION: AtomicUsize = AtomicUsize::new(0); - - #[op] - fn op_promise_reject() -> Result<(), AnyError> { - PROMISE_REJECT.fetch_add(1, Ordering::Relaxed); - Ok(()) - } - - #[op] - fn op_uncaught_exception() -> Result<(), AnyError> { - UNCAUGHT_EXCEPTION.fetch_add(1, Ordering::Relaxed); - Ok(()) - } - - let extension = Extension::builder() - .ops(vec![ - op_promise_reject::decl(), - op_uncaught_exception::decl(), - ]) - .build(); - - #[derive(Default)] - struct ModsLoader; - - impl ModuleLoader for ModsLoader { - fn resolve( - &self, - specifier: &str, - referrer: &str, - _is_main: bool, - ) -> Result { - assert_eq!(specifier, "file:///main.js"); - assert_eq!(referrer, "."); - let s = crate::resolve_import(specifier, referrer).unwrap(); - Ok(s) - } - - fn load( - &self, - _module_specifier: &ModuleSpecifier, - _maybe_referrer: Option, - _is_dyn_import: bool, - ) -> Pin> { - let source = r#" - Deno.core.opSync("op_set_promise_reject_callback", (type, promise, reason) => { - Deno.core.opSync("op_promise_reject"); - throw reason; - }); - - Deno.core.opSync("op_set_uncaught_exception_callback", (err) => { - Deno.core.opSync("op_uncaught_exception"); - }); - - throw new Error('top level throw'); - "#; - - async move { - Ok(ModuleSource { - code: source.as_bytes().to_vec().into_boxed_slice(), - module_url_specified: "file:///main.js".to_string(), - module_url_found: "file:///main.js".to_string(), - module_type: ModuleType::JavaScript, - }) - } - .boxed_local() - } - } - - let mut runtime = JsRuntime::new(RuntimeOptions { - extensions: vec![extension], - module_loader: Some(Rc::new(ModsLoader)), - ..Default::default() - }); - - let id = runtime - .load_main_module(&crate::resolve_url("file:///main.js").unwrap(), None) - .await - .unwrap(); - let receiver = runtime.mod_evaluate(id); - runtime.run_event_loop(false).await.unwrap(); - receiver.await.unwrap().unwrap(); - - assert_eq!(1, PROMISE_REJECT.load(Ordering::Relaxed)); - assert_eq!(1, UNCAUGHT_EXCEPTION.load(Ordering::Relaxed)); - } - #[test] fn test_op_return_serde_v8_error() { #[op]