fix(napi): promise resolve error (#1664)

This commit is contained in:
LongYinan 2023-07-24 00:36:24 +08:00 committed by GitHub
parent 4fdce25a17
commit a7eeb0c31c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 212 additions and 214 deletions

View file

@ -124,7 +124,7 @@ jobs:
- settings:
target: i686-pc-windows-msvc
node: 18
name: stable - ${{ matrix.settings.host }} - node@${{ matrix.node }}
name: ${{ matrix.settings.host }} - node@${{ matrix.node }} - toolchain@ ${{ matrix.settings.toolchain }}
runs-on: ${{ matrix.settings.host }}
steps:
@ -292,6 +292,8 @@ jobs:
options: -v ${{ github.workspace }}/.cargo-cache/registry:/usr/local/cargo/registry -v ${{ github.workspace }}/.cargo-cache/git:/usr/local/cargo/git -v ${{ github.workspace }}:/napi-rs -w /napi-rs
run: |
yarn build:test -- --target ${{ matrix.settings.target }}
chmod 777 -R .cargo-cache
chmod 777 -R target
- uses: actions/upload-artifact@v3
with:
@ -365,8 +367,16 @@ jobs:
uses: addnab/docker-run-action@v3
with:
image: ${{ steps.image-name.outputs.docker-image }}
options: ${{ matrix.settings.args }} -v ${{ github.workspace }}:/build -w /build
run: yarn test
options: ${{ matrix.settings.args }} -v ${{ github.workspace }}/cores:/cores -v ${{ github.workspace }}:/build -w /build
run: >-
ulimit -c &&
ulimit -c unlimited &&
ulimit -c &&
yarn test
- name: List files
run: |
ls -la .
ls -la ./cores
build-and-test-linux-armv7:
name: stable - armv7-unknown-linux-gnu - node@18

View file

@ -72,11 +72,8 @@ pub fn run<T: Task>(
env,
raw_resource,
async_work_name,
Some(execute::<T> as unsafe extern "C" fn(env: sys::napi_env, data: *mut c_void)),
Some(
complete::<T>
as unsafe extern "C" fn(env: sys::napi_env, status: sys::napi_status, data: *mut c_void),
),
Some(execute::<T>),
Some(complete::<T>),
(result as *mut AsyncWork<T>).cast(),
&mut result.napi_async_work,
)

View file

@ -81,7 +81,10 @@ impl<T: FromNapiValue> ValidateNapiValue for Promise<T> {
unsafe { crate::sys::napi_create_error(env, code, message, &mut err) },
"Failed to create rejected error"
)?;
check_status!(unsafe { crate::sys::napi_reject_deferred(env, deferred, err) })?;
check_status!(
unsafe { crate::sys::napi_reject_deferred(env, deferred, err) },
"Failed to reject promise in validate"
)?;
return Ok(promise);
}
Ok(ptr::null_mut())

View file

@ -18,7 +18,7 @@ impl TypeName for String {
impl ValidateNapiValue for String {}
impl ToNapiValue for String {
impl ToNapiValue for &String {
unsafe fn to_napi_value(env: sys::napi_env, val: Self) -> Result<sys::napi_value> {
let mut ptr = ptr::null_mut();
@ -31,6 +31,16 @@ impl ToNapiValue for String {
}
}
impl ToNapiValue for String {
#[inline]
unsafe fn to_napi_value(env: sys::napi_env, val: Self) -> Result<sys::napi_value> {
#[allow(clippy::needless_borrow)]
unsafe {
ToNapiValue::to_napi_value(env, &val)
}
}
}
impl FromNapiValue for String {
unsafe fn from_napi_value(env: sys::napi_env, napi_val: sys::napi_value) -> Result<Self> {
let mut len = 0;

View file

@ -159,5 +159,5 @@ unsafe extern "C" fn async_task_abort_controller_finalize(
finalize_data: *mut c_void,
_finalize_hint: *mut c_void,
) {
unsafe { Box::from_raw(finalize_data as *mut AbortSignal) };
drop(unsafe { Box::from_raw(finalize_data as *mut AbortSignal) });
}

View file

@ -268,7 +268,7 @@ impl<T: 'static, S: 'static> SharedReference<T, S> {
let raw = Box::into_raw(Box::new(s));
let prev_drop_fn = unsafe { Box::from_raw(self.owner.finalize_callbacks.get()) };
let drop_fn = Box::new(move || {
unsafe { Box::from_raw(raw) };
drop(unsafe { Box::from_raw(raw) });
prev_drop_fn();
});
self.owner.finalize_callbacks.set(Box::into_raw(drop_fn));

View file

@ -8,7 +8,7 @@ use std::os::raw::{c_char, c_void};
use std::ptr;
use crate::bindgen_runtime::FromNapiValue;
#[cfg(all(feature = "napi4"))]
#[cfg(feature = "napi4")]
use crate::bindgen_runtime::ToNapiValue;
use crate::{
async_work::{self, AsyncWorkPromise},
@ -23,15 +23,15 @@ use crate::{
use crate::async_cleanup_hook::AsyncCleanupHook;
#[cfg(feature = "napi3")]
use crate::cleanup_env::{CleanupEnvHook, CleanupEnvHookData};
#[cfg(all(feature = "serde-json"))]
#[cfg(feature = "serde-json")]
use crate::js_values::{De, Ser};
#[cfg(feature = "napi4")]
use crate::threadsafe_function::{ThreadSafeCallContext, ThreadsafeFunction};
#[cfg(feature = "napi3")]
use crate::JsError;
#[cfg(all(feature = "serde-json"))]
#[cfg(feature = "serde-json")]
use serde::de::DeserializeOwned;
#[cfg(all(feature = "serde-json"))]
#[cfg(feature = "serde-json")]
use serde::Serialize;
pub type Callback = unsafe extern "C" fn(sys::napi_env, sys::napi_callback_info) -> sys::napi_value;
@ -1345,7 +1345,7 @@ pub(crate) unsafe extern "C" fn raw_finalize<T>(
finalize_hint: *mut c_void,
) {
let tagged_object = finalize_data as *mut TaggedObject<T>;
unsafe { Box::from_raw(tagged_object) };
drop(unsafe { Box::from_raw(tagged_object) });
if !finalize_hint.is_null() {
let size_hint = unsafe { *Box::from_raw(finalize_hint as *mut Option<i64>) };
if let Some(changed) = size_hint {

View file

@ -1,6 +1,6 @@
use std::convert::{From, TryFrom};
use std::error;
use std::ffi::{CStr, CString};
use std::ffi::CString;
use std::fmt;
#[cfg(feature = "serde-json")]
use std::fmt::Display;
@ -25,8 +25,7 @@ pub struct Error<S: AsRef<str> = Status> {
pub status: S,
pub reason: String,
// Convert raw `JsError` into Error
maybe_raw: sys::napi_ref,
maybe_env: sys::napi_env,
pub(crate) maybe_raw: sys::napi_ref,
}
impl<S: AsRef<str>> ToNapiValue for Error<S> {
@ -36,9 +35,14 @@ impl<S: AsRef<str>> ToNapiValue for Error<S> {
Ok(err)
} else {
let mut value = std::ptr::null_mut();
check_status!(unsafe {
sys::napi_get_reference_value(val.maybe_env, val.maybe_raw, &mut value)
})?;
check_status!(
unsafe { sys::napi_get_reference_value(env, val.maybe_raw, &mut value) },
"Get error reference in `to_napi_value` failed"
)?;
check_status!(
unsafe { sys::napi_delete_reference(env, val.maybe_raw) },
"Delete error reference in `to_napi_value` failed"
)?;
Ok(value)
}
}
@ -79,7 +83,7 @@ impl From<SerdeJSONError> for Error {
impl From<JsUnknown> for Error {
fn from(value: JsUnknown) -> Self {
let mut result = std::ptr::null_mut();
let status = unsafe { sys::napi_create_reference(value.0.env, value.0.value, 0, &mut result) };
let status = unsafe { sys::napi_create_reference(value.0.env, value.0.value, 1, &mut result) };
if status != sys::Status::napi_ok {
return Error::new(
Status::from(status),
@ -90,7 +94,6 @@ impl From<JsUnknown> for Error {
status: Status::GenericFailure,
reason: "".to_string(),
maybe_raw: result,
maybe_env: value.0.env,
}
}
}
@ -118,7 +121,6 @@ impl<S: AsRef<str>> Error<S> {
status,
reason: reason.to_string(),
maybe_raw: ptr::null_mut(),
maybe_env: ptr::null_mut(),
}
}
@ -127,7 +129,6 @@ impl<S: AsRef<str>> Error<S> {
status,
reason: "".to_owned(),
maybe_raw: ptr::null_mut(),
maybe_env: ptr::null_mut(),
}
}
}
@ -138,7 +139,6 @@ impl Error {
status: Status::GenericFailure,
reason: reason.into(),
maybe_raw: ptr::null_mut(),
maybe_env: ptr::null_mut(),
}
}
}
@ -149,7 +149,6 @@ impl From<std::ffi::NulError> for Error {
status: Status::GenericFailure,
reason: format!("{}", error),
maybe_raw: ptr::null_mut(),
maybe_env: ptr::null_mut(),
}
}
}
@ -160,23 +159,6 @@ impl From<std::io::Error> for Error {
status: Status::GenericFailure,
reason: format!("{}", error),
maybe_raw: ptr::null_mut(),
maybe_env: ptr::null_mut(),
}
}
}
impl<S: AsRef<str>> Drop for Error<S> {
fn drop(&mut self) {
#[cfg(not(feature = "noop"))]
{
if !self.maybe_env.is_null() && !self.maybe_raw.is_null() {
let delete_reference_status =
unsafe { sys::napi_delete_reference(self.maybe_env, self.maybe_raw) };
debug_assert!(
delete_reference_status == sys::Status::napi_ok,
"Delete Error Reference failed"
);
}
}
}
}
@ -237,6 +219,11 @@ macro_rules! impl_object_methods {
get_err_status == sys::Status::napi_ok,
"Get Error from Reference failed"
);
let delete_err_status = unsafe { sys::napi_delete_reference(env, self.0.maybe_raw) };
debug_assert!(
delete_err_status == sys::Status::napi_ok,
"Delete Error Reference failed"
);
return err;
}
@ -289,28 +276,6 @@ macro_rules! impl_object_methods {
status
);
}
#[allow(clippy::not_unsafe_ptr_arg_deref)]
pub fn throw(&self, env: sys::napi_env) -> Result<()> {
let error_status = format!("{:?}\0", self.0.status.as_ref());
let status_len = error_status.len();
let error_code_string =
unsafe { CStr::from_bytes_with_nul_unchecked(error_status.as_bytes()) };
let reason_len = self.0.reason.len();
let reason_c_string = format!("{}\0", self.0.reason.clone());
let reason = unsafe { CStr::from_bytes_with_nul_unchecked(reason_c_string.as_bytes()) };
let mut error_code = ptr::null_mut();
let mut reason_string = ptr::null_mut();
let mut js_error = ptr::null_mut();
check_status!(unsafe {
sys::napi_create_string_utf8(env, error_code_string.as_ptr(), status_len, &mut error_code)
})?;
check_status!(unsafe {
sys::napi_create_string_utf8(env, reason.as_ptr(), reason_len, &mut reason_string)
})?;
check_status!(unsafe { $kind(env, error_code, reason_string, &mut js_error) })?;
check_status!(unsafe { sys::napi_throw(env, js_error) })
}
}
impl<S: AsRef<str>> From<Error<S>> for $js_value<S> {

View file

@ -4,8 +4,6 @@ use std::os::raw::c_void;
use std::ptr;
use crate::bindgen_runtime::{ToNapiValue, THREAD_DESTROYED};
#[cfg(feature = "deferred_trace")]
use crate::JsError;
use crate::{check_status, JsObject, Value};
use crate::{sys, Env, Error, Result};
#[cfg(feature = "deferred_trace")]
@ -19,92 +17,73 @@ use crate::{NapiRaw, NapiValue};
///
/// See this issue for more details:
/// https://github.com/nodejs/node-addon-api/issues/595
struct DeferredTrace {
value: sys::napi_ref,
#[cfg(not(feature = "noop"))]
env: sys::napi_env,
}
#[repr(transparent)]
struct DeferredTrace(sys::napi_ref);
#[cfg(feature = "deferred_trace")]
impl DeferredTrace {
fn new(raw_env: sys::napi_env) -> Self {
fn new(raw_env: sys::napi_env) -> Result<Self> {
let env = unsafe { Env::from_raw(raw_env) };
let reason = env.create_string("none").unwrap();
let mut js_error = ptr::null_mut();
let create_error_status =
unsafe { sys::napi_create_error(raw_env, ptr::null_mut(), reason.raw(), &mut js_error) };
debug_assert!(create_error_status == sys::Status::napi_ok);
check_status!(
unsafe { sys::napi_create_error(raw_env, ptr::null_mut(), reason.raw(), &mut js_error) },
"Create error in DeferredTrace failed"
)?;
let mut result = ptr::null_mut();
let status = unsafe { sys::napi_create_reference(raw_env, js_error, 1, &mut result) };
debug_assert!(status == sys::Status::napi_ok);
check_status!(
unsafe { sys::napi_create_reference(raw_env, js_error, 1, &mut result) },
"Create reference in DeferredTrace failed"
)?;
Self {
value: result,
#[cfg(not(feature = "noop"))]
env: raw_env,
}
Ok(Self(result))
}
fn into_rejected(self, raw_env: sys::napi_env, err: Error) -> Result<sys::napi_value> {
let env = unsafe { Env::from_raw(raw_env) };
let raw = unsafe { DeferredTrace::to_napi_value(raw_env, self)? };
let mut raw = ptr::null_mut();
check_status!(
unsafe { sys::napi_get_reference_value(raw_env, self.0, &mut raw) },
"Failed to get referenced value in DeferredTrace"
)?;
let mut obj = unsafe { JsObject::from_raw(raw_env, raw)? };
if err.reason.is_empty() && err.status == crate::Status::GenericFailure {
// Can't clone err as the clone containing napi pointers will
// be freed when this function returns, causing err to be freed twice.
// Someone should probably fix this.
let err_obj = JsError::from(err).into_unknown(env).coerce_to_object()?;
let mut obj = unsafe { JsObject::from_raw_unchecked(raw_env, raw) };
let err_value = if !err.maybe_raw.is_null() {
let mut err_raw_value = std::ptr::null_mut();
check_status!(
unsafe { sys::napi_get_reference_value(raw_env, err.maybe_raw, &mut err_raw_value) },
"Get error reference in `to_napi_value` failed"
)?;
let err_obj = unsafe { JsObject::from_raw_unchecked(raw_env, err_raw_value) };
if err_obj.has_named_property("message")? {
let err_value = if err_obj.has_named_property("message")? {
// The error was already created inside the JS engine, just return it
Ok(unsafe { JsError::from(Error::from(err_obj.into_unknown())).into_value(raw_env) })
Ok(unsafe { err_obj.raw() })
} else {
obj.set_named_property("message", "")?;
obj.set_named_property("code", "")?;
Ok(raw)
}
};
check_status!(
unsafe { sys::napi_delete_reference(raw_env, err.maybe_raw) },
"Delete error reference in `to_napi_value` failed"
)?;
err_value
} else {
obj.set_named_property("message", env.create_string(&err.reason)?)?;
obj.set_named_property("code", env.create_string_from_std(err.status.to_string())?)?;
obj.set_named_property("message", &err.reason)?;
obj.set_named_property(
"code",
env.create_string_from_std(format!("{}", err.status))?,
)?;
Ok(raw)
}
}
}
#[cfg(feature = "deferred_trace")]
impl ToNapiValue for DeferredTrace {
unsafe fn to_napi_value(env: sys::napi_env, val: Self) -> Result<sys::napi_value> {
let mut value = ptr::null_mut();
check_status!(unsafe { sys::napi_get_reference_value(env, val.value, &mut value) })?;
if value.is_null() {
// This shouldn't happen but a panic is better than a segfault
Err(Error::new(
crate::Status::GenericFailure,
"Failed to get deferred error reference",
))
} else {
Ok(value)
}
}
}
#[cfg(feature = "deferred_trace")]
impl Drop for DeferredTrace {
fn drop(&mut self) {
#[cfg(not(feature = "noop"))]
{
if !self.env.is_null() && !self.value.is_null() {
let delete_reference_status = unsafe { sys::napi_delete_reference(self.env, self.value) };
debug_assert!(
delete_reference_status == sys::Status::napi_ok,
"Delete Error Reference failed"
);
}
}
};
check_status!(
unsafe { sys::napi_delete_reference(raw_env, self.0) },
"Failed to get referenced value in DeferredTrace"
)?;
err_value
}
}
@ -138,12 +117,14 @@ impl<Data: ToNapiValue, Resolver: FnOnce(Env) -> Result<Data>> JsDeferred<Data,
// Create a threadsafe function so we can call back into the JS thread when we are done.
let mut async_resource_name = ptr::null_mut();
let s = unsafe { CStr::from_bytes_with_nul_unchecked(b"napi_resolve_deferred\0") };
check_status!(unsafe {
sys::napi_create_string_utf8(env, s.as_ptr(), 22, &mut async_resource_name)
})?;
check_status!(
unsafe { sys::napi_create_string_utf8(env, s.as_ptr(), 22, &mut async_resource_name) },
"Create async resource name in JsDeferred failed"
)?;
let mut tsfn = ptr::null_mut();
check_status! {unsafe {
check_status!(
unsafe {
sys::napi_create_threadsafe_function(
env,
ptr::null_mut(),
@ -153,16 +134,18 @@ impl<Data: ToNapiValue, Resolver: FnOnce(Env) -> Result<Data>> JsDeferred<Data,
1,
ptr::null_mut(),
None,
raw_deferred as *mut c_void,
raw_deferred.cast(),
Some(napi_resolve_deferred::<Data, Resolver>),
&mut tsfn,
)
}}?;
},
"Create threadsafe function in JsDeferred failed"
)?;
let deferred = Self {
tsfn,
#[cfg(feature = "deferred_trace")]
trace: DeferredTrace::new(env),
trace: DeferredTrace::new(env)?,
_data: PhantomData,
_resolver: PhantomData,
};
@ -198,13 +181,13 @@ impl<Data: ToNapiValue, Resolver: FnOnce(Env) -> Result<Data>> JsDeferred<Data,
let status = unsafe {
sys::napi_call_threadsafe_function(
self.tsfn,
Box::into_raw(Box::from(data)) as *mut c_void,
sys::ThreadsafeFunctionCallMode::nonblocking,
Box::into_raw(Box::from(data)).cast(),
sys::ThreadsafeFunctionCallMode::blocking,
)
};
debug_assert!(
status == sys::Status::napi_ok,
"Call threadsafe function failed"
"Call threadsafe function in JsDeferred failed"
);
let status = unsafe {
@ -212,7 +195,7 @@ impl<Data: ToNapiValue, Resolver: FnOnce(Env) -> Result<Data>> JsDeferred<Data,
};
debug_assert!(
status == sys::Status::napi_ok,
"Release threadsafe function failed"
"Release threadsafe function in JsDeferred failed"
);
}
}
@ -229,34 +212,46 @@ extern "C" fn napi_resolve_deferred<Data: ToNapiValue, Resolver: FnOnce(Env) ->
return;
}
}
let deferred = context as sys::napi_deferred;
let deferred_data = unsafe { Box::from_raw(data as *mut DeferredData<Data, Resolver>) };
let deferred = context.cast();
let deferred_data: Box<DeferredData<Data, Resolver>> = unsafe { Box::from_raw(data.cast()) };
let result = deferred_data
.resolver
.and_then(|resolver| resolver(unsafe { Env::from_raw(env) }))
.and_then(|res| unsafe { ToNapiValue::to_napi_value(env, res) });
match result {
Ok(res) => {
let status = unsafe { sys::napi_resolve_deferred(env, deferred, res) };
debug_assert!(
status == sys::Status::napi_ok,
"Resolve promise failed {:?}",
crate::Status::from(status)
);
}
Err(e) => {
if let Err(e) = result.and_then(|res| {
check_status!(
unsafe { sys::napi_resolve_deferred(env, deferred, res) },
"Resolve deferred value failed"
)
}) {
#[cfg(feature = "deferred_trace")]
let error = deferred_data.trace.into_rejected(env, e).unwrap();
let error = deferred_data.trace.into_rejected(env, e);
#[cfg(not(feature = "deferred_trace"))]
let error = unsafe { crate::JsError::from(e).into_value(env) };
let error = Ok::<sys::napi_value, Error>(unsafe { crate::JsError::from(e).into_value(env) });
let status = unsafe { sys::napi_reject_deferred(env, deferred, error) };
debug_assert!(
status == sys::Status::napi_ok,
"Reject promise failed {:?}",
crate::Status::from(status)
match error {
Ok(error) => {
unsafe { sys::napi_reject_deferred(env, deferred, error) };
}
Err(err) => {
#[cfg(debug_assertions)]
{
println!("Failed to reject deferred: {:?}", err);
let mut err = ptr::null_mut();
let mut err_msg = ptr::null_mut();
unsafe {
sys::napi_create_string_utf8(
env,
"Rejection failed\0".as_ptr().cast(),
0,
&mut err_msg,
);
sys::napi_create_error(env, ptr::null_mut(), err_msg, &mut err);
sys::napi_reject_deferred(env, deferred, err);
}
}
}
}
}
}

View file

@ -337,9 +337,12 @@ macro_rules! impl_object_methods {
&mut js_function,
)
})?;
check_status!(unsafe {
check_status!(
unsafe {
sys::napi_set_named_property(self.0.env, self.0.value, name.as_ptr(), js_function)
})
},
"create_named_method error"
)
}
pub fn get_named_property<T>(&self, name: &str) -> Result<T>
@ -348,9 +351,12 @@ macro_rules! impl_object_methods {
{
let key = CString::new(name)?;
let mut raw_value = ptr::null_mut();
check_status!(unsafe {
check_status!(
unsafe {
sys::napi_get_named_property(self.0.env, self.0.value, key.as_ptr(), &mut raw_value)
})?;
},
"get_named_property error"
)?;
unsafe { <T as FromNapiValue>::from_napi_value(self.0.env, raw_value) }
}
@ -360,18 +366,24 @@ macro_rules! impl_object_methods {
{
let key = CString::new(name)?;
let mut raw_value = ptr::null_mut();
check_status!(unsafe {
check_status!(
unsafe {
sys::napi_get_named_property(self.0.env, self.0.value, key.as_ptr(), &mut raw_value)
})?;
},
"get_named_property_unchecked error"
)?;
unsafe { <T as FromNapiValue>::from_napi_value(self.0.env, raw_value) }
}
pub fn has_named_property(&self, name: &str) -> Result<bool> {
pub fn has_named_property<N: AsRef<str>>(&self, name: N) -> Result<bool> {
let mut result = false;
let key = CString::new(name)?;
check_status!(unsafe {
let key = CString::new(name.as_ref())?;
check_status!(
unsafe {
sys::napi_has_named_property(self.0.env, self.0.value, key.as_ptr(), &mut result)
})?;
},
"napi_has_named_property error"
)?;
Ok(result)
}

View file

@ -488,7 +488,8 @@ impl<T: 'static> ThreadsafeFunction<T, ErrorStrategy::CalleeHandled> {
return Err(crate::Error::from_status(Status::Closing));
}
check_status!(unsafe {
check_status!(
unsafe {
sys::napi_call_threadsafe_function(
self.handle.get_raw(),
Box::into_raw(Box::new(value.map(|data| {
@ -507,7 +508,9 @@ impl<T: 'static> ThreadsafeFunction<T, ErrorStrategy::CalleeHandled> {
.cast(),
ThreadsafeFunctionCallMode::NonBlocking.into(),
)
})
},
"Threadsafe function call_async failed"
)
})?;
receiver
.await
@ -741,11 +744,12 @@ unsafe extern "C" fn call_js_cb<T: 'static, V: ToNapiValue, R, ES>(
err
);
let message_length = message.len();
let c_message = CString::new(message).unwrap();
unsafe {
sys::napi_fatal_error(
"threadsafe_function.rs:720\0".as_ptr().cast(),
"threadsafe_function.rs:749\0".as_ptr().cast(),
26,
CString::new(message).unwrap().into_raw(),
c_message.as_ptr(),
message_length,
)
};

View file

@ -50,6 +50,7 @@ struct JsClassForEither {}
#[napi]
impl JsClassForEither {
#[napi(constructor)]
#[allow(clippy::new_without_default)]
pub fn new() -> Self {
JsClassForEither {}
}
@ -60,6 +61,7 @@ struct AnotherClassForEither {}
#[napi]
impl AnotherClassForEither {
#[allow(clippy::new_without_default)]
#[napi(constructor)]
pub fn new() -> Self {
Self {}