archive-gh-me/napi-rs
1
0
Fork 0
mirror of https://github.com/napi-rs/napi-rs.git synced 2025-12-08 19:56:07 +00:00
Code Issues Projects Releases Wiki Activity

fix!(napi): unsound issues in Error (#2644)

Browse Source 
- Close https://github.com/napi-rs/napi-rs/issues/1640
This commit is contained in:
LongYinan 2025-05-17 22:44:24 +08:00 committed by GitHub
parent fddf6109b0
commit b1fb82dade
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 81 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
crates/macro/src/expand/napi.rs
View File

@ -36,7 +36,6 @@ fn dtor() {
if let Ok(f) = fs::OpenOptions::new()
.read(true)
.append(true)
.write(true)
.open(type_def_file)
{
let mut writer = BufWriter::<fs::File>::new(f);

1
crates/napi/src/bindgen_runtime/js_values/stream/read.rs
View File

@ -498,7 +498,6 @@ impl<T: FromNapiValue + 'static> futures_core::Stream for Reader<T> {
reason: "".to_string(),
maybe_raw: error_ref,
maybe_env: cx.env.0,
raw: true,
});
Ok(())
})?

11
crates/napi/src/bindgen_runtime/mod.rs
View File

@ -53,11 +53,12 @@ pub(crate) unsafe extern "C" fn raw_finalize_unchecked<T: ObjectFinalize>(
// If `Rc` strong count is 2, it means the finalize of referenced `Object` is called before the `fn drop` of the `Reference`
// It always happened on exiting process
// In general, the `fn drop` would happen first
assert!(
rc_strong_count == 1 || rc_strong_count == 2,
"Rc strong count is: {}, it should be 1 or 2",
rc_strong_count
);
if rc_strong_count != 1 && rc_strong_count != 2 {
eprintln!(
"Rc strong count is: {}, it should be 1 or 2",
rc_strong_count
);
}
}
let finalize = unsafe { Box::from_raw(finalize_callbacks_rc.get()) };
finalize();

84
crates/napi/src/error.rs
View File

@ -25,7 +25,26 @@ pub struct Error<S: AsRef<str> = Status> {
// Convert raw `JsError` into Error
pub(crate) maybe_raw: sys::napi_ref,
pub(crate) maybe_env: sys::napi_env,
pub(crate) raw: bool,
}
impl<S: AsRef<str>> Drop for Error<S> {
fn drop(&mut self) {
// @TODO: deal with Error created with reference and leave it to drop in `async fn`
if !self.maybe_raw.is_null() {
let mut ref_count = 0;
let status =
unsafe { sys::napi_reference_unref(self.maybe_env, self.maybe_raw, &mut ref_count) };
if status != sys::Status::napi_ok {
eprintln!("unref error reference failed: {}", Status::from(status));
}
if ref_count == 0 {
let status = unsafe { sys::napi_delete_reference(self.maybe_env, self.maybe_raw) };
if status != sys::Status::napi_ok {
eprintln!("delete error reference failed: {}", Status::from(status));
}
}
}
}
}
impl<S: AsRef<str>> std::fmt::Debug for Error<S> {
@ -39,20 +58,8 @@ impl<S: AsRef<str>> std::fmt::Debug for Error<S> {
}
}
impl<S: Clone + AsRef<str>> Clone for Error<S> {
fn clone(&self) -> Self {
Self {
raw: false,
status: self.status.clone(),
reason: self.reason.to_string(),
maybe_raw: self.maybe_raw,
maybe_env: self.maybe_env,
}
}
}
impl<S: AsRef<str>> ToNapiValue for Error<S> {
unsafe fn to_napi_value(env: sys::napi_env, val: Self) -> Result<sys::napi_value> {
unsafe fn to_napi_value(env: sys::napi_env, mut val: Self) -> Result<sys::napi_value> {
if val.maybe_raw.is_null() {
let err = unsafe { JsError::from(val).into_value(env) };
Ok(err)
@ -62,12 +69,20 @@ impl<S: AsRef<str>> ToNapiValue for Error<S> {
unsafe { sys::napi_get_reference_value(env, val.maybe_raw, &mut value) },
"Get error reference in `to_napi_value` failed"
)?;
if val.raw {
let mut ref_count = 0;
check_status!(
unsafe { sys::napi_reference_unref(env, val.maybe_raw, &mut ref_count) },
"Unref error reference in `to_napi_value` failed"
)?;
if ref_count == 0 {
check_status!(
unsafe { sys::napi_delete_reference(env, val.maybe_raw) },
"Delete error reference in `to_napi_value` failed"
)?;
}
// already unref, skip the logic in `Drop`
val.maybe_raw = ptr::null_mut();
val.maybe_env = ptr::null_mut();
Ok(value)
}
}
@ -115,7 +130,6 @@ impl From<Unknown<'_>> for Error {
"Create Error reference failed".to_owned(),
);
}
let maybe_env = value.0.env;
let maybe_error_message = value
.coerce_to_string()
@ -126,7 +140,6 @@ impl From<Unknown<'_>> for Error {
reason: error_message,
maybe_raw: result,
maybe_env,
raw: true,
};
}
@ -135,7 +148,6 @@ impl From<Unknown<'_>> for Error {
reason: "".to_string(),
maybe_raw: result,
maybe_env,
raw: true,
}
}
}
@ -164,7 +176,6 @@ impl<S: AsRef<str>> Error<S> {
reason: reason.to_string(),
maybe_raw: ptr::null_mut(),
maybe_env: ptr::null_mut(),
raw: true,
}
}
@ -174,11 +185,27 @@ impl<S: AsRef<str>> Error<S> {
reason: "".to_owned(),
maybe_raw: ptr::null_mut(),
maybe_env: ptr::null_mut(),
raw: true,
}
}
}
impl<S: AsRef<str> + Clone> Error<S> {
pub fn try_clone(&self) -> Result<Self> {
if !self.maybe_raw.is_null() {
check_status!(
unsafe { sys::napi_reference_ref(self.maybe_env, self.maybe_raw, &mut 0) },
"Failed to increase error reference count"
)?;
}
Ok(Self {
status: self.status.clone(),
reason: self.reason.to_string(),
maybe_raw: self.maybe_raw,
maybe_env: self.maybe_env,
})
}
}
impl Error {
pub fn from_reason<T: Into<String>>(reason: T) -> Self {
Error {
@ -186,7 +213,6 @@ impl Error {
reason: reason.into(),
maybe_raw: ptr::null_mut(),
maybe_env: ptr::null_mut(),
raw: true,
}
}
}
@ -198,7 +224,6 @@ impl From<std::ffi::NulError> for Error {
reason: format!("{}", error),
maybe_raw: ptr::null_mut(),
maybe_env: ptr::null_mut(),
raw: true,
}
}
}
@ -210,7 +235,6 @@ impl From<std::io::Error> for Error {
reason: format!("{}", error),
maybe_raw: ptr::null_mut(),
maybe_env: ptr::null_mut(),
raw: true,
}
}
}
@ -262,7 +286,7 @@ macro_rules! impl_object_methods {
/// # Safety
///
/// This function is safety if env is not null ptr.
pub unsafe fn into_value(self, env: sys::napi_env) -> sys::napi_value {
pub unsafe fn into_value(mut self, env: sys::napi_env) -> sys::napi_value {
if !self.0.maybe_raw.is_null() {
let mut err = ptr::null_mut();
let get_err_status =
@ -271,13 +295,23 @@ macro_rules! impl_object_methods {
get_err_status == sys::Status::napi_ok,
"Get Error from Reference failed"
);
if self.0.raw {
let mut ref_count = 0;
let unref_status =
unsafe { sys::napi_reference_unref(env, self.0.maybe_raw, &mut ref_count) };
debug_assert!(
unref_status == sys::Status::napi_ok,
"Unref Error Reference failed"
);
if ref_count == 0 {
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"
);
}
// already unref, skip the logic in `Drop`
self.0.maybe_raw = ptr::null_mut();
self.0.maybe_env = ptr::null_mut();
let mut is_error = false;
let is_error_status = unsafe { sys::napi_is_error(env, err, &mut is_error) };
debug_assert!(

16
crates/napi/src/js_values/deferred.rs
View File

@ -42,7 +42,7 @@ impl DeferredTrace {
Ok(Self(result))
}
fn into_rejected(self, raw_env: sys::napi_env, err: Error) -> Result<sys::napi_value> {
fn into_rejected(self, raw_env: sys::napi_env, mut err: Error) -> Result<sys::napi_value> {
let env = Env::from_raw(raw_env);
let mut raw = ptr::null_mut();
check_status!(
@ -67,10 +67,20 @@ impl DeferredTrace {
obj.set_named_property("code", "")?;
Ok(raw)
};
let mut ref_count = 0;
check_status!(
unsafe { sys::napi_delete_reference(raw_env, err.maybe_raw) },
"Delete error reference in `to_napi_value` failed"
unsafe { sys::napi_reference_unref(raw_env, err.maybe_raw, &mut ref_count) },
"Unref error reference in `to_napi_value` failed"
)?;
if ref_count == 0 {
check_status!(
unsafe { sys::napi_delete_reference(raw_env, err.maybe_raw) },
"Delete error reference in `to_napi_value` failed"
)?;
}
// already unref, skip the logic in `Drop`
err.maybe_env = ptr::null_mut();
err.maybe_raw = ptr::null_mut();
err_value
} else {
obj.set_named_property("message", &err.reason)?;

1
crates/napi/src/threadsafe_function.rs
View File

@ -688,7 +688,6 @@ unsafe extern "C" fn call_js_cb<
Err(Error {
maybe_raw: error_reference,
maybe_env: raw_env,
raw: true,
status: Status::from(status),
reason: "".to_owned(),
})

4
examples/napi/src/error.rs
View File

@ -61,9 +61,9 @@ impl CustomStruct {
}
#[napi]
pub fn js_error_callback(value: Unknown) -> Vec<JsError> {
pub fn js_error_callback(value: Unknown) -> Result<Vec<JsError>> {
let error: Error = value.into();
vec![error.clone().into(), error.into()]
Ok(vec![error.try_clone()?.into(), error.into()])
}
#[napi]

2
examples/napi/src/stream.rs
View File

@ -13,7 +13,7 @@ pub fn accept_stream(
let mut input = StreamReader::new(web_readable_stream.map(|chunk| {
chunk
.map(|chunk| bytes::Bytes::copy_from_slice(&chunk))
.map_err(|e| std::io::Error::other(e.reason))
.map_err(|e| std::io::Error::other(e.reason.clone()))
}));
AsyncBlockBuilder::build_with_map(
env,