Wrapper over DeferredPtr

Author: adespawn

src/async_bridge.rs

@@ -17,6 +17,7 @@ use napi::{Env, Error, Result, Status, sys};
 use napi_derive::napi;
 
 use crate::errors::{ConvertedError, ConvertedResult, JsResult, with_custom_error_sync};
+use crate::napi_helpers::{DeferredPtr, ResolveOrReject};
 
 /// JsPromise — lightweight wrapper over the promise pointer that indicates the type used to resolve the promise
 /// The promise can be either resolved with type T or rejected with any error value (`ConvertedError` when used with `submit_future`).
@@ -30,14 +31,14 @@ impl<T> ToNapiValue for JsPromise<T> {
     }
 }
 
-type SettleCallback = Box<dyn FnOnce(Env, sys::napi_deferred) + Send>;
+type SettleCallback = Box<dyn FnOnce(Env, DeferredPtr) + Send>;
 type BridgedFuture = Pin<Box<dyn Future<Output = SettleCallback> + Send>>;
 
 struct FutureEntry {
     future: BridgedFuture,
     /// Raw deferred handle — resolved/rejected in `poll_woken` on the
     /// main thread where we have a valid `napi_env`.
-    deferred: sys::napi_deferred,
+    deferred: DeferredPtr,
     waker: Waker,
 }
 
@@ -122,12 +123,7 @@ impl FutureRegistry {
         }
     }
 
-    fn insert(
-        &mut self,
-        env: &Env,
-        future: BridgedFuture,
-        deferred: sys::napi_deferred,
-    ) -> Result<u64> {
+    fn insert(&mut self, env: &Env, future: BridgedFuture, deferred: DeferredPtr) -> Result<u64> {
         let was_empty = self.futures.is_empty();
 
         let id = self.next_id;
@@ -234,7 +230,7 @@ thread_local! {
   static REGISTRY: RefCell<FutureRegistry> = RefCell::new(FutureRegistry::new());
 }
 
-fn create_promise(env: &Env) -> Result<(sys::napi_deferred, sys::napi_value)> {
+fn create_promise(env: &Env) -> Result<(DeferredPtr, sys::napi_value)> {
     let mut deferred = ptr::null_mut();
     let mut promise = ptr::null_mut();
     // SAFETY: `raw_env` is taken from Env, which is guaranteed to be valid for the lifetime of the current napi call.
@@ -245,21 +241,21 @@ fn create_promise(env: &Env) -> Result<(sys::napi_deferred, sys::napi_value)> {
             &mut promise
         ))?
     };
-    Ok((deferred, promise))
+    // SAFETY: deferred is assigned to valid value in napi_create_promise call, that have just succeeded.
+    // This promise had no chance to be resolved yet.
+    let deferred_ptr = unsafe { DeferredPtr::new(deferred) };
+    Ok((deferred_ptr, promise))
 }
 
-/// # Safety
-/// The deferred must not have been resolved or rejected yet
-unsafe fn reject_with_reason(env: Env, deferred: sys::napi_deferred, reason: &str) -> Result<()> {
+fn reject_with_reason(env: Env, deferred: DeferredPtr, reason: &str) -> Result<()> {
     // We can unwrap in the second place, because the only case when Cstring::new can fail is when the string contains a null byte.
     let c_reason = CString::new(reason).unwrap_or_else(|_| {
         CString::new("[Unknown error] Error message contained illegal null byte").unwrap()
     });
     let mut msg: sys::napi_value = std::ptr::null_mut();
     let mut error: sys::napi_value = std::ptr::null_mut();
 
-    // SAFETY: Env guarantees that raw pointer is a valid main-thread env and
-    // caller ensured that `deferred` has not yet been resolved or rejected.
+    // SAFETY: Env guarantees that raw pointer is a valid main-thread env.
     // Remaining arguments are created in this function and are valid for the whole duration.
     unsafe {
         check_status!(sys::napi_create_string_utf8(
@@ -274,7 +270,7 @@ unsafe fn reject_with_reason(env: Env, deferred: sys::napi_deferred, reason: &st
             msg,
             &mut error
         ))?;
-        check_status!(sys::napi_reject_deferred(env.raw(), deferred, error))?;
+        deferred.resolve(env, error, ResolveOrReject::Reject)?;
     }
     Ok(())
 }
@@ -360,33 +356,26 @@ where
 
     let boxed: BridgedFuture = Box::pin(async move {
         let result = fut.await;
-        Box::new(move |env: Env, deferred| unsafe {
+        Box::new(move |env: Env, deferred: DeferredPtr| unsafe {
             // SAFETY: This closure is only ever invoked from `poll_woken`, which runs
             // on the Node main thread inside the TSFN callback - the only place where
             // `env` is a valid napi_env. `deferred` is consumed exactly once here,
             // satisfying the napi contract that each deferred is resolved or rejected
             // exactly once. `to_napi_value` receives the same valid `env`.
             let (js_val, resolve) = match result {
-                Ok(val) => (T::to_napi_value(env.raw(), val), true),
-                Err(err) => (ConvertedError::to_napi_value(env.raw(), err), false),
+                Ok(val) => (T::to_napi_value(env.raw(), val), ResolveOrReject::Resolve),
+                Err(err) => (
+                    ConvertedError::to_napi_value(env.raw(), err),
+                    ResolveOrReject::Reject,
+                ),
+            };
+
+            let status = match js_val {
+                Ok(v) => deferred.resolve(env, v, resolve),
+                Err(e) => reject_with_reason(env, deferred, &e.reason),
             };
-            let status = js_val
-                // First we try to accept / reject with converted value / error.
-                .and_then(|v| {
-                    if resolve {
-                        check_status!(sys::napi_resolve_deferred(env.raw(), deferred, v))
-                    } else {
-                        check_status!(sys::napi_reject_deferred(env.raw(), deferred, v))
-                    }
-                })
-                // If this fails, or we failed to convert the value / error into a JS value,
-                // we reject with a fallback reason.
-                .or_else(|e| reject_with_reason(env, deferred, &e.reason));
 
             if let Err(e) = status {
-                // If both fail, we assume something terrible has happened. We cannot
-                // inform JS side about the error by regular error handling, so we panic to
-                // avoid silent failures and orphaned promises.
                 panic!(
                     "Failed to settle promise in TSFN callback. This may indicate either a bug in the driver or a severe runtime error.\nRoot cause:\n {}",
                     e.reason

src/lib.rs

@@ -5,6 +5,7 @@ extern crate napi_derive;
 pub mod async_bridge;
 pub mod errors;
 pub mod metadata;
+pub mod napi_helpers;
 pub mod options;
 pub mod paging;
 pub mod requests;

src/napi_helpers.rs

@@ -0,0 +1,50 @@
+use std::{marker::PhantomData, rc::Rc};
+
+use napi::{Env, Result, bindgen_prelude::check_status, sys};
+
+/// Wrapper over napi_deferred pointer, that ensures safe usage of the pointer and prevents double resolve/reject.
+pub(crate) struct DeferredPtr {
+    ptr: sys::napi_deferred,
+    // We want to block DeferredPtr from being Send or Sync,
+    // as we can use napi_deferred pointer can be used only in the main nodejs thread.
+    _not_send_sync: PhantomData<Rc<()>>,
+}
+
+pub(crate) enum ResolveOrReject {
+    Resolve,
+    Reject,
+}
+
+impl DeferredPtr {
+    /// # Safety
+    /// The pointer must not have been resolved or rejected yet, and must point to a valid napi_deferred.
+    pub(crate) unsafe fn new(ptr: sys::napi_deferred) -> Self {
+        Self {
+            ptr,
+            _not_send_sync: PhantomData,
+        }
+    }
+
+    /// # Safety
+    /// Valid pointer to value must be provided
+    pub(crate) unsafe fn resolve(
+        self,
+        env: Env,
+        value: sys::napi_value,
+        mode: ResolveOrReject,
+    ) -> Result<()> {
+        // We can use the napi_deferred only once, as per napi documentation,
+        // any calls to resolve it, will free the value: https://nodejs.org/api/n-api.html#promises
+        // While there is no specification what happens if the call fails, it's safer to assume
+        // the pointer is no longer valid, and we are in non-recoverable state.
+
+        // SAFETY: Constraints of this class ensure validity of the deref pointer,
+        // and Env ensures validity of the napi_env.
+        // Caller ensures validity of the value pointer.
+        if let ResolveOrReject::Resolve = mode {
+            unsafe { check_status!(sys::napi_resolve_deferred(env.raw(), self.ptr, value)) }
+        } else {
+            unsafe { check_status!(sys::napi_reject_deferred(env.raw(), self.ptr, value)) }
+        }
+    }
+}