From 752ffea1d9445fe3a474f00c430c8d3ef4b9a252 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 10 Apr 2023 17:02:47 +0800 Subject: [PATCH] fix(napi): revert Promise changes because of the flaky test --- .github/workflows/linux-armv7.yaml | 1 + .../src/bindgen_runtime/js_values/promise.rs | 95 +++++++------------ crates/napi/src/js_values/deferred.rs | 12 ++- 3 files changed, 46 insertions(+), 62 deletions(-) diff --git a/.github/workflows/linux-armv7.yaml b/.github/workflows/linux-armv7.yaml index 173b0456..5042cd3c 100644 --- a/.github/workflows/linux-armv7.yaml +++ b/.github/workflows/linux-armv7.yaml @@ -2,6 +2,7 @@ name: Linux-armv7 env: DEBUG: 'napi:*' + RUST_BACKTRACE: 1 concurrency: group: ${{ github.workflow }}-${{ github.ref }} diff --git a/crates/napi/src/bindgen_runtime/js_values/promise.rs b/crates/napi/src/bindgen_runtime/js_values/promise.rs index 3605c5fa..09fe1fd8 100644 --- a/crates/napi/src/bindgen_runtime/js_values/promise.rs +++ b/crates/napi/src/bindgen_runtime/js_values/promise.rs @@ -1,33 +1,30 @@ -use std::ffi::{c_void, CStr}; +use std::ffi::CStr; use std::future; use std::pin::Pin; use std::ptr; use std::sync::{ - atomic::{AtomicBool, AtomicPtr, Ordering}, + atomic::{AtomicBool, Ordering}, Arc, }; -use std::task::{Context, Poll, Waker}; +use std::task::{Context, Poll}; -use crate::{check_status, sys, Error, JsUnknown, NapiValue, Result}; +use tokio::sync::oneshot::{channel, Receiver, Sender}; + +use crate::{check_status, sys, Error, JsUnknown, NapiValue, Result, Status}; use super::{FromNapiValue, TypeName, ValidateNapiValue}; -struct PromiseInner { - value: AtomicPtr>, - waker: AtomicPtr, - aborted: AtomicBool, +pub struct Promise { + value: Pin>>>, + aborted: Arc, } -impl Drop for PromiseInner { +impl Drop for Promise { fn drop(&mut self) { self.aborted.store(true, Ordering::SeqCst); } } -pub struct Promise { - inner: Arc>, -} - impl TypeName for Promise { fn type_name() -> &'static str { "Promise" @@ -103,13 +100,9 @@ impl FromNapiValue for Promise { )?; let mut promise_after_then = ptr::null_mut(); let mut then_js_cb = ptr::null_mut(); - let promise_inner = PromiseInner { - value: AtomicPtr::new(ptr::null_mut()), - waker: AtomicPtr::new(ptr::null_mut()), - aborted: AtomicBool::new(false), - }; - let shared_inner = Arc::new(promise_inner); - let context_ptr = Arc::into_raw(shared_inner.clone()); + let (tx, rx) = channel(); + let aborted = Arc::new(AtomicBool::new(false)); + let tx_ptr = Box::into_raw(Box::new((tx, aborted.clone()))); check_status!( unsafe { sys::napi_create_function( @@ -117,7 +110,7 @@ impl FromNapiValue for Promise { then_c_string.as_ptr(), 4, Some(then_callback::), - context_ptr as *mut c_void, + tx_ptr.cast(), &mut then_js_cb, ) }, @@ -152,7 +145,7 @@ impl FromNapiValue for Promise { catch_c_string.as_ptr(), 5, Some(catch_callback::), - context_ptr as *mut c_void, + tx_ptr.cast(), &mut catch_js_cb, ) }, @@ -172,7 +165,8 @@ impl FromNapiValue for Promise { "Failed to call catch method" )?; Ok(Promise { - inner: shared_inner, + value: Box::pin(rx), + aborted, }) } } @@ -180,19 +174,13 @@ impl FromNapiValue for Promise { impl future::Future for Promise { type Output = Result; - fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { - if self.inner.value.load(Ordering::Relaxed).is_null() { - if self.inner.waker.load(Ordering::Acquire).is_null() { - self.inner.waker.store( - Box::into_raw(Box::new(cx.waker().clone())), - Ordering::Release, - ); - } - Poll::Pending - } else { - Poll::Ready( - unsafe { Box::from_raw(self.inner.value.load(Ordering::Relaxed)) }.map_err(Error::from), - ) + fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + match self.value.as_mut().poll(cx) { + Poll::Pending => Poll::Pending, + Poll::Ready(v) => Poll::Ready( + v.map_err(|e| Error::new(Status::GenericFailure, format!("{}", e))) + .and_then(|v| unsafe { *Box::from_raw(v) }.map_err(Error::from)), + ), } } } @@ -218,20 +206,15 @@ unsafe extern "C" fn then_callback( get_cb_status == sys::Status::napi_ok, "Get callback info from Promise::then failed" ); - let PromiseInner { - value, - waker, - aborted, - } = &*unsafe { Arc::from_raw(data as *mut PromiseInner) }; + let (sender, aborted) = + *unsafe { Box::from_raw(data as *mut (Sender<*mut Result>, Arc)) }; if aborted.load(Ordering::SeqCst) { return this; } let resolve_value_t = Box::new(unsafe { T::from_napi_value(env, resolved_value[0]) }); - value.store(Box::into_raw(resolve_value_t), Ordering::Relaxed); - let waker = waker.load(Ordering::Acquire); - if !waker.is_null() { - unsafe { Box::from_raw(waker) }.wake(); - } + sender + .send(Box::into_raw(resolve_value_t)) + .expect("Send Promise resolved value error"); this } @@ -258,23 +241,15 @@ unsafe extern "C" fn catch_callback( "Get callback info from Promise::catch failed" ); let rejected_value = rejected_value[0]; - let PromiseInner { - value, - waker, - aborted, - } = &*unsafe { Arc::from_raw(data as *mut PromiseInner) }; + let (sender, aborted) = + *unsafe { Box::from_raw(data as *mut (Sender<*mut Result>, Arc)) }; if aborted.load(Ordering::SeqCst) { return this; } - value.store( - Box::into_raw(Box::new(Err(Error::from(unsafe { + sender + .send(Box::into_raw(Box::new(Err(Error::from(unsafe { JsUnknown::from_raw_unchecked(env, rejected_value) - })))), - Ordering::Relaxed, - ); - let waker = waker.load(Ordering::Acquire); - if !waker.is_null() { - unsafe { Box::from_raw(waker) }.wake(); - } + }))))) + .expect("Send Promise resolved value error"); this } diff --git a/crates/napi/src/js_values/deferred.rs b/crates/napi/src/js_values/deferred.rs index 2660e283..23ac087a 100644 --- a/crates/napi/src/js_values/deferred.rs +++ b/crates/napi/src/js_values/deferred.rs @@ -115,12 +115,20 @@ extern "C" fn napi_resolve_deferred match result { Ok(res) => { let status = unsafe { sys::napi_resolve_deferred(env, deferred, res) }; - debug_assert!(status == sys::Status::napi_ok, "Resolve promise failed"); + debug_assert!( + status == sys::Status::napi_ok, + "Resolve promise failed {:?}", + crate::Status::from(status) + ); } Err(e) => { let status = unsafe { sys::napi_reject_deferred(env, deferred, JsError::from(e).into_value(env)) }; - debug_assert!(status == sys::Status::napi_ok, "Reject promise failed"); + debug_assert!( + status == sys::Status::napi_ok, + "Reject promise failed {:?}", + crate::Status::from(status) + ); } } }