Skip to content

Commit

Permalink
refactor(core): remove unneeded ops for uncaughtException (denoland#1…
Browse files Browse the repository at this point in the history 
…5296)
  • Loading branch information
@bartlomieju
bartlomieju committed Jul 24, 2022
1 parent 7036600 commit 2e1d6d3
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 108 deletions.
46 changes: 1 addition & 45 deletions core/bindings.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -324,8 +324,6 @@ pub extern "C" fn promise_reject_callback(message: v8::PromiseRejectMessage) {
let mut state = state_rc.borrow_mut();

if let Some(js_promise_reject_cb) = state.js_promise_reject_cb.clone() {
let js_uncaught_exception_cb = state.js_uncaught_exception_cb.clone();

let tc_scope = &mut v8::TryCatch::new(scope);
let undefined: v8::Local<v8::Value> = v8::undefined(tc_scope).into();
let type_ = v8::Integer::new(tc_scope, message.get_event() as i32);
Expand Down Expand Up @@ -358,52 +356,10 @@ pub extern "C" fn promise_reject_callback(message: v8::PromiseRejectMessage) {
if !pending_mod_evaluate.has_evaluated {
pending_mod_evaluate
.handled_promise_rejections
.push(promise_global.clone());
.push(promise_global);
}
}
}

// TODO(bartlomieju): remove this whole block, `js_uncaught_exception_cb` is
// not needed anymore
if let Some(exception) = tc_scope.exception() {
if let Some(js_uncaught_exception_cb) = js_uncaught_exception_cb {
tc_scope.reset(); // Cancel pending exception.
{
let mut state = state_rc.borrow_mut();
if let Some(pending_mod_evaluate) =
state.pending_mod_evaluate.as_mut()
{
if !pending_mod_evaluate.has_evaluated {
pending_mod_evaluate
.handled_promise_rejections
.push(promise_global);
}
}
}
js_uncaught_exception_cb.open(tc_scope).call(
tc_scope,
undefined,
&[exception],
);
}
}

if tc_scope.has_caught() {
// TODO(bartlomieju): ensure that TODO provided below is still valid.
// If we get here, an exception was thrown by the unhandledRejection
// handler and there is ether no uncaughtException handler or the
// handler threw an exception of its own.
//
// TODO(bnoordhuis) Node terminates the process or worker thread
// but we don't really have that option. The exception won't bubble
// up either because V8 cancels it when this function returns.
let exception = tc_scope
.stack_trace()
.or_else(|| tc_scope.exception())
.map(|value| value.to_rust_string_lossy(tc_scope))
.unwrap_or_else(|| "no exception".into());
eprintln!("Unhandled exception: {}", exception);
}
} else {
let promise = message.get_promise();
let promise_global = v8::Global::new(scope, promise);
Expand Down
13 changes: 0 additions & 13 deletions core/ops_builtin_v8.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ pub(crate) fn init_builtins_v8() -> Vec<OpDecl> {
op_set_macrotask_callback::decl(),
op_set_next_tick_callback::decl(),
op_set_promise_reject_callback::decl(),
op_set_uncaught_exception_callback::decl(),
op_run_microtasks::decl(),
op_has_tick_scheduled::decl(),
op_set_has_tick_scheduled::decl(),
Expand Down Expand Up @@ -109,18 +108,6 @@ fn op_set_promise_reject_callback<'a>(
Ok(old.map(|v| from_v8(scope, v.into()).unwrap()))
}

#[op(v8)]
fn op_set_uncaught_exception_callback<'a>(
scope: &mut v8::HandleScope<'a>,
cb: serde_v8::Value,
) -> Result<Option<serde_v8::Value<'a>>, Error> {
let cb = to_v8_fn(scope, cb)?;
let state_rc = JsRuntime::state(scope);
let old = state_rc.borrow_mut().js_uncaught_exception_cb.replace(cb);
let old = old.map(|v| v8::Local::new(scope, v));
Ok(old.map(|v| from_v8(scope, v.into()).unwrap()))
}

#[op(v8)]
fn op_run_microtasks(scope: &mut v8::HandleScope) {
scope.perform_microtask_checkpoint();
Expand Down
56 changes: 6 additions & 50 deletions core/runtime.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,6 @@ pub(crate) struct JsRuntimeState {
pub(crate) js_macrotask_cbs: Vec<v8::Global<v8::Function>>,
pub(crate) js_nexttick_cbs: Vec<v8::Global<v8::Function>>,
pub(crate) js_promise_reject_cb: Option<v8::Global<v8::Function>>,
pub(crate) js_uncaught_exception_cb: Option<v8::Global<v8::Function>>,
pub(crate) js_format_exception_cb: Option<v8::Global<v8::Function>>,
pub(crate) has_tick_scheduled: bool,
pub(crate) js_wasm_streaming_cb: Option<v8::Global<v8::Function>>,
Expand Down Expand Up @@ -394,7 +393,6 @@ impl JsRuntime {
js_macrotask_cbs: vec![],
js_nexttick_cbs: vec![],
js_promise_reject_cb: None,
js_uncaught_exception_cb: None,
js_format_exception_cb: None,
has_tick_scheduled: false,
js_wasm_streaming_cb: None,
Expand Down Expand Up @@ -3211,25 +3209,15 @@ assertEquals(1, notify_return_value);
#[tokio::test]
async fn test_set_promise_reject_callback() {
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(),
])
.ops(vec![op_promise_reject::decl()])
.build();

let mut runtime = JsRuntime::new(RuntimeOptions {
Expand All @@ -3249,23 +3237,17 @@ assertEquals(1, notify_return_value);
if (reason.message !== "reject") {
throw Error("unexpected reason: " + reason);
}
Deno.core.opSync("op_store_pending_promise_exception", promise);
Deno.core.opSync("op_promise_reject");
throw Error("promiseReject"); // Triggers uncaughtException handler.
});
Deno.core.opSync("op_set_uncaught_exception_callback", (err) => {
if (err.message !== "promiseReject") throw err;
Deno.core.opSync("op_uncaught_exception");
});
new Promise((_, reject) => reject(Error("reject")));
"#,
)
.unwrap();
runtime.run_event_loop(false).await.unwrap();
runtime.run_event_loop(false).await.unwrap_err();

assert_eq!(1, PROMISE_REJECT.load(Ordering::Relaxed));
assert_eq!(1, UNCAUGHT_EXCEPTION.load(Ordering::Relaxed));

runtime
.execute_script(
Expand All @@ -3277,47 +3259,27 @@ assertEquals(1, notify_return_value);
});
}
{
const prev = Deno.core.opSync("op_set_uncaught_exception_callback", (...args) => {
prev(...args);
throw Error("fail");
});
}
new Promise((_, reject) => reject(Error("reject")));
"#,
)
.unwrap();
// Exception from uncaughtException handler doesn't bubble up but is
// printed to stderr.
runtime.run_event_loop(false).await.unwrap();
runtime.run_event_loop(false).await.unwrap_err();

assert_eq!(2, PROMISE_REJECT.load(Ordering::Relaxed));
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(),
])
.ops(vec![op_promise_reject::decl()])
.build();

#[derive(Default)]
Expand Down Expand Up @@ -3345,11 +3307,6 @@ assertEquals(1, notify_return_value);
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');
Expand Down Expand Up @@ -3379,10 +3336,9 @@ assertEquals(1, notify_return_value);
.unwrap();
let receiver = runtime.mod_evaluate(id);
runtime.run_event_loop(false).await.unwrap();
receiver.await.unwrap().unwrap();
receiver.await.unwrap().unwrap_err();

assert_eq!(1, PROMISE_REJECT.load(Ordering::Relaxed));
assert_eq!(1, UNCAUGHT_EXCEPTION.load(Ordering::Relaxed));
}

#[test]
Expand Down

0 comments on commit 2e1d6d3

Please sign in to comment.