0
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-10-30 09:08:00 -04:00

fix(core): always report the first error on unhandled rejection (#18992)

The root cause of denoland/deno_std#3320, I believe, is that
`pending_promise_rejections` is a `HashMap` whose entries are in
arbitrary order, and as a result either of the two errors (`AddrInUse`
and `TypeError`) may be selected when determining which one to report. I
changed the field to a `VecDeque` so that the first error (`AddrInUse`
in this case) is always selected.
This commit is contained in:
ud2 2023-05-08 06:27:59 +08:00 committed by GitHub
parent 7e1ae65572
commit 40987178c4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 27 additions and 22 deletions

View file

@ -499,12 +499,12 @@ pub extern "C" fn promise_reject_callback(message: v8::PromiseRejectMessage) {
let error_global = v8::Global::new(scope, error); let error_global = v8::Global::new(scope, error);
context_state context_state
.pending_promise_rejections .pending_promise_rejections
.insert(promise_global, error_global); .push_back((promise_global, error_global));
} }
PromiseHandlerAddedAfterReject => { PromiseHandlerAddedAfterReject => {
context_state context_state
.pending_promise_rejections .pending_promise_rejections
.remove(&promise_global); .retain(|(key, _)| key != &promise_global);
} }
PromiseRejectAfterResolved => {} PromiseRejectAfterResolved => {}
PromiseResolveAfterResolved => { PromiseResolveAfterResolved => {

View file

@ -894,7 +894,7 @@ fn op_store_pending_promise_rejection<'a>(
let error_global = v8::Global::new(scope, reason.v8_value); let error_global = v8::Global::new(scope, reason.v8_value);
context_state context_state
.pending_promise_rejections .pending_promise_rejections
.insert(promise_global, error_global); .push_back((promise_global, error_global));
} }
#[op(v8)] #[op(v8)]
@ -909,7 +909,7 @@ fn op_remove_pending_promise_rejection<'a>(
let promise_global = v8::Global::new(scope, promise_value); let promise_global = v8::Global::new(scope, promise_value);
context_state context_state
.pending_promise_rejections .pending_promise_rejections
.remove(&promise_global); .retain(|(key, _)| key != &promise_global);
} }
#[op(v8)] #[op(v8)]
@ -924,7 +924,8 @@ fn op_has_pending_promise_rejection<'a>(
let promise_global = v8::Global::new(scope, promise_value); let promise_global = v8::Global::new(scope, promise_value);
context_state context_state
.pending_promise_rejections .pending_promise_rejections
.contains_key(&promise_global) .iter()
.any(|(key, _)| key == &promise_global)
} }
#[op(v8)] #[op(v8)]

View file

@ -7,8 +7,8 @@ use crate::runtime::exception_to_err_result;
use crate::JsRuntime; use crate::JsRuntime;
use anyhow::Error; use anyhow::Error;
use std::cell::RefCell; use std::cell::RefCell;
use std::collections::HashMap;
use std::collections::HashSet; use std::collections::HashSet;
use std::collections::VecDeque;
use std::hash::BuildHasherDefault; use std::hash::BuildHasherDefault;
use std::hash::Hasher; use std::hash::Hasher;
use std::option::Option; use std::option::Option;
@ -43,7 +43,7 @@ pub(crate) struct ContextState {
pub(crate) js_format_exception_cb: Option<Rc<v8::Global<v8::Function>>>, pub(crate) js_format_exception_cb: Option<Rc<v8::Global<v8::Function>>>,
pub(crate) js_wasm_streaming_cb: Option<Rc<v8::Global<v8::Function>>>, pub(crate) js_wasm_streaming_cb: Option<Rc<v8::Global<v8::Function>>>,
pub(crate) pending_promise_rejections: pub(crate) pending_promise_rejections:
HashMap<v8::Global<v8::Promise>, v8::Global<v8::Value>>, VecDeque<(v8::Global<v8::Promise>, v8::Global<v8::Value>)>,
pub(crate) unrefed_ops: HashSet<i32, BuildHasherDefault<IdentityHasher>>, pub(crate) unrefed_ops: HashSet<i32, BuildHasherDefault<IdentityHasher>>,
// We don't explicitly re-read this prop but need the slice to live alongside // We don't explicitly re-read this prop but need the slice to live alongside
// the context // the context
@ -270,22 +270,9 @@ impl<'s> JsRealmLocal<'s> {
let context_state_rc = self.state(scope); let context_state_rc = self.state(scope);
let mut context_state = context_state_rc.borrow_mut(); let mut context_state = context_state_rc.borrow_mut();
if context_state.pending_promise_rejections.is_empty() { let Some((_, handle)) = context_state.pending_promise_rejections.pop_front() else {
return Ok(()); return Ok(());
}
let key = {
context_state
.pending_promise_rejections
.keys()
.next()
.unwrap()
.clone()
}; };
let handle = context_state
.pending_promise_rejections
.remove(&key)
.unwrap();
drop(context_state); drop(context_state);
let exception = v8::Local::new(scope, handle); let exception = v8::Local::new(scope, handle);

View file

@ -1831,7 +1831,7 @@ impl JsRuntime {
.state(tc_scope) .state(tc_scope)
.borrow_mut() .borrow_mut()
.pending_promise_rejections .pending_promise_rejections
.remove(&promise_global); .retain(|(key, _)| key != &promise_global);
} }
} }
let promise_global = v8::Global::new(tc_scope, promise); let promise_global = v8::Global::new(tc_scope, promise);
@ -4138,6 +4138,23 @@ Deno.core.opAsync("op_async_serialize_object_with_numbers_as_keys", {
.contains("JavaScript execution has been terminated")); .contains("JavaScript execution has been terminated"));
} }
#[tokio::test]
async fn test_unhandled_rejection_order() {
let mut runtime = JsRuntime::new(Default::default());
runtime
.execute_script_static(
"",
r#"
for (let i = 0; i < 100; i++) {
Promise.reject(i);
}
"#,
)
.unwrap();
let err = runtime.run_event_loop(false).await.unwrap_err();
assert_eq!(err.to_string(), "Uncaught (in promise) 0");
}
#[tokio::test] #[tokio::test]
async fn test_set_promise_reject_callback() { async fn test_set_promise_reject_callback() {
static PROMISE_REJECT: AtomicUsize = AtomicUsize::new(0); static PROMISE_REJECT: AtomicUsize = AtomicUsize::new(0);