Skip to content

Commit

Permalink
feat(neon): Update try-catch to match the RFC
Browse files Browse the repository at this point in the history
  • Loading branch information
kjvalencik committed Dec 7, 2020
1 parent 5f6481a commit 8beb98e
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 48 deletions.
6 changes: 3 additions & 3 deletions crates/neon-sys/native/src/neon.cc
Original file line number Diff line number Diff line change
Expand Up @@ -542,10 +542,10 @@ extern "C" void Neon_EventHandler_Delete(void * thread_safe_cb) {
cb->close();
}

extern "C" try_catch_control_t Neon_TryCatch_With(Neon_TryCatchGlue glue_fn, void *rust_thunk, void *cx, v8::Local<v8::Value> *result, void **unwind_value) {
extern "C" try_catch_control_t Neon_TryCatch_With(Neon_TryCatchGlue glue_fn, void *rust_thunk, void *cx, void *ok, v8::Local<v8::Value> *err, void **unwind_value) {
Nan::TryCatch try_catch;

try_catch_control_t ctrl = glue_fn(rust_thunk, cx, result, unwind_value);
try_catch_control_t ctrl = glue_fn(rust_thunk, cx, ok, unwind_value);

if (ctrl == CONTROL_PANICKED) {
return CONTROL_PANICKED;
Expand All @@ -560,7 +560,7 @@ extern "C" try_catch_control_t Neon_TryCatch_With(Neon_TryCatchGlue glue_fn, voi
}
return CONTROL_RETURNED;
} else {
*result = try_catch.Exception();
*err = try_catch.Exception();
return CONTROL_THREW;
}
}
4 changes: 2 additions & 2 deletions crates/neon-sys/native/src/neon.h
Original file line number Diff line number Diff line change
Expand Up @@ -152,13 +152,13 @@ extern "C" {
// returns `CONTROL_RETURNED`.
// The `unwind_value` out-parameter can be assumed to be initialized if and only if this
// function returns `CONTROL_PANICKED`.
typedef try_catch_control_t (*Neon_TryCatchGlue)(void *rust_thunk, void *cx, v8::Local<v8::Value> *result, void **unwind_value);
typedef try_catch_control_t (*Neon_TryCatchGlue)(void *rust_thunk, void *cx, void *ok, void **unwind_value);

// The `result` out-parameter can be assumed to be initialized if and only if this function
// returns `CONTROL_RETURNED` or `CONTROL_THREW`.
// The `unwind_value` out-parameter can be assumed to be initialized if and only if this
// function returns `CONTROL_PANICKED`.
try_catch_control_t Neon_TryCatch_With(Neon_TryCatchGlue glue, void *rust_thunk, void *cx, v8::Local<v8::Value> *result, void **unwind_value);
try_catch_control_t Neon_TryCatch_With(Neon_TryCatchGlue glue, void *rust_thunk, void *cx, void *ok, v8::Local<v8::Value> *err, void **unwind_value);
}

#endif
5 changes: 3 additions & 2 deletions crates/neon-sys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ pub struct InheritedHandleScope;
/// glue function returns `TryCatchControl::Panicked`.
pub type TryCatchGlue = extern fn(rust_thunk: *mut c_void,
cx: *mut c_void,
result: *mut Local,
result: *mut c_void,
unwind_value: *mut *mut c_void) -> TryCatchControl;

extern "C" {
Expand Down Expand Up @@ -230,6 +230,7 @@ extern "C" {
pub fn Neon_TryCatch_With(glue: TryCatchGlue,
rust_thunk: *mut c_void,
cx: *mut c_void,
result: *mut Local,
ok: *mut c_void,
err: *mut Local,
unwind_value: *mut *mut c_void) -> TryCatchControl;
}
41 changes: 20 additions & 21 deletions src/context/internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@ use std::cell::Cell;
use std::mem::MaybeUninit;
use std::os::raw::c_void;
#[cfg(feature = "legacy-runtime")]
use std::panic::{AssertUnwindSafe, UnwindSafe, catch_unwind, resume_unwind};
use std::panic::{AssertUnwindSafe, catch_unwind, resume_unwind};
use neon_runtime;
use neon_runtime::raw;
use neon_runtime::scope::Root;
#[cfg(feature = "legacy-runtime")]
use neon_runtime::try_catch::TryCatchControl;
use types::{JsObject, JsValue, Value};
use types::{JsObject, JsValue};
use handle::Handle;
use object::class::ClassMap;
use result::{JsResult, NeonResult};
use result::NeonResult;
use super::ModuleContext;

#[cfg(feature = "legacy-runtime")]
Expand Down Expand Up @@ -126,22 +126,24 @@ pub trait ContextInternal<'a>: Sized {
fn deactivate(&self) { self.scope_metadata().active.set(false); }

#[cfg(feature = "legacy-runtime")]
fn try_catch_internal<'b: 'a, T, F>(&mut self, f: F) -> Result<Handle<'a, T>, Handle<'a, JsValue>>
where T: Value,
F: UnwindSafe + FnOnce(&mut Self) -> JsResult<'b, T>
fn try_catch_internal<'b: 'a, T, F>(&mut self, f: F) -> Result<T, Handle<'a, JsValue>>
where T: Sized,
F: FnOnce(&mut Self) -> NeonResult<T>,
{
// A closure does not have a guaranteed layout, so we need to box it in order to pass
// a pointer to it across the boundary into C++.
let rust_thunk = Box::into_raw(Box::new(f));

let mut local: MaybeUninit<raw::Local> = MaybeUninit::zeroed();
let mut ok: MaybeUninit<T> = MaybeUninit::zeroed();
let mut err: MaybeUninit<raw::Local> = MaybeUninit::zeroed();
let mut unwind_value: MaybeUninit<*mut c_void> = MaybeUninit::zeroed();

let ctrl = unsafe {
neon_runtime::try_catch::with(try_catch_glue::<Self, T, F>,
rust_thunk as *mut c_void,
(self as *mut Self) as *mut c_void,
local.as_mut_ptr(),
ok.as_mut_ptr() as *mut c_void,
err.as_mut_ptr(),
unwind_value.as_mut_ptr())
};

Expand All @@ -152,13 +154,10 @@ pub trait ContextInternal<'a>: Sized {
};
resume_unwind(unwind_value);
}
TryCatchControl::Returned => {
let local = unsafe { local.assume_init() };
Ok(Handle::new_internal(T::from_raw(self.env(), local)))
}
TryCatchControl::Returned => Ok(unsafe { ok.assume_init() }),
TryCatchControl::Threw => {
let local = unsafe { local.assume_init() };
Err(JsValue::new_internal(local))
let err = unsafe { err.assume_init() };
Err(JsValue::new_internal(err))
}
TryCatchControl::UnexpectedErr => {
panic!("try_catch: unexpected Err(Throw) when VM is not in a throwing state");
Expand All @@ -167,9 +166,9 @@ pub trait ContextInternal<'a>: Sized {
}

#[cfg(feature = "napi-runtime")]
fn try_catch_internal<'b: 'a, T, F>(&mut self, f: F) -> Result<Handle<'a, T>, Handle<'a, JsValue>>
where T: Value,
F: FnOnce(&mut Self) -> JsResult<'b, T>
fn try_catch_internal<'b: 'a, T, F>(&mut self, f: F) -> Result<T, Handle<'a, JsValue>>
where T: Sized,
F: FnOnce(&mut Self) -> NeonResult<T>
{
let result = f(self);
let mut local: MaybeUninit<raw::Local> = MaybeUninit::zeroed();
Expand All @@ -188,11 +187,11 @@ pub trait ContextInternal<'a>: Sized {
#[cfg(feature = "legacy-runtime")]
extern "C" fn try_catch_glue<'a, 'b: 'a, C, T, F>(rust_thunk: *mut c_void,
cx: *mut c_void,
returned: *mut raw::Local,
returned: *mut c_void,
unwind_value: *mut *mut c_void) -> TryCatchControl
where C: ContextInternal<'a>,
T: Value,
F: UnwindSafe + FnOnce(&mut C) -> JsResult<'b, T>
T: Sized,
F: FnOnce(&mut C) -> NeonResult<T>,
{
let f: F = *unsafe { Box::from_raw(rust_thunk as *mut F) };
let cx: &mut C = unsafe { std::mem::transmute(cx) };
Expand All @@ -205,7 +204,7 @@ extern "C" fn try_catch_glue<'a, 'b: 'a, C, T, F>(rust_thunk: *mut c_void,
match catch_unwind(AssertUnwindSafe(|| f(cx))) {
// No Rust panic, no JS exception.
Ok(Ok(result)) => unsafe {
*returned = result.to_raw();
(returned as *mut T).write(result);
TryCatchControl::Returned
}
// No Rust panic, caught a JS exception.
Expand Down
16 changes: 4 additions & 12 deletions src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,18 +242,10 @@ pub trait Context<'a>: ContextInternal<'a> {
result
}

#[cfg(all(feature = "try-catch-api", feature = "napi-runtime"))]
fn try_catch<'b: 'a, T, F>(&mut self, f: F) -> Result<Handle<'a, T>, Handle<'a, JsValue>>
where T: Value,
F: FnOnce(&mut Self) -> JsResult<'b, T>
{
self.try_catch_internal(f)
}

#[cfg(all(feature = "try-catch-api", feature = "legacy-runtime"))]
fn try_catch<'b: 'a, T, F>(&mut self, f: F) -> Result<Handle<'a, T>, Handle<'a, JsValue>>
where T: Value,
F: UnwindSafe + FnOnce(&mut Self) -> JsResult<'b, T>
#[cfg(feature = "try-catch-api")]
fn try_catch<'b: 'a, T, F>(&mut self, f: F) -> Result<T, Handle<'a, JsValue>>
where T: Sized,
F: FnOnce(&mut Self) -> NeonResult<T>
{
self.try_catch_internal(f)
}
Expand Down
8 changes: 4 additions & 4 deletions test/dynamic/native/src/js/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,10 @@ pub fn compute_scoped(mut cx: FunctionContext) -> JsResult<JsNumber> {

pub fn throw_and_catch(mut cx: FunctionContext) -> JsResult<JsValue> {
let v = cx.argument_opt(0).unwrap_or_else(|| cx.undefined().upcast());
Ok(cx.try_catch(|cx| {
let _ = cx.throw(v)?;
Ok(cx.string("unreachable").upcast())
}).unwrap_or_else(|err| err))

cx.try_catch(|cx| cx.throw(v))
.map(|_: ()| Ok(cx.string("unreachable").upcast()))
.unwrap_or_else(|err| Ok(err))
}

pub fn call_and_catch(mut cx: FunctionContext) -> JsResult<JsValue> {
Expand Down
8 changes: 4 additions & 4 deletions test/napi/native/src/js/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,10 @@ pub fn compute_scoped(mut cx: FunctionContext) -> JsResult<JsNumber> {

pub fn throw_and_catch(mut cx: FunctionContext) -> JsResult<JsValue> {
let v = cx.argument_opt(0).unwrap_or_else(|| cx.undefined().upcast());
Ok(cx.try_catch(|cx| {
let _ = cx.throw(v)?;
Ok(cx.string("unreachable").upcast())
}).unwrap_or_else(|err| err))

cx.try_catch(|cx| cx.throw(v))
.map(|_: ()| Ok(cx.string("unreachable").upcast()))
.unwrap_or_else(|err| Ok(err))
}

pub fn call_and_catch(mut cx: FunctionContext) -> JsResult<JsValue> {
Expand Down

0 comments on commit 8beb98e

Please sign in to comment.