fix(neon): Fix undefined behavior in channel callbacks when there is …

…a panic

* Catches panics and converts to uncaughtException
* Throws uncaughtException if throwing
* Passes opaque exceptions through as JsBox

crates/neon-runtime/src/napi/bindings/functions.rs

@@ -226,6 +226,24 @@ mod napi1 {
             fn create_promise(env: Env, deferred: *mut Deferred, promise: *mut Value) -> Status;
             fn resolve_deferred(env: Env, deferred: Deferred, resolution: Value) -> Status;
             fn reject_deferred(env: Env, deferred: Deferred, rejection: Value) -> Status;
+
+            fn fatal_error(
+                location: *const c_char,
+                location_len: usize,
+                message: *const c_char,
+                message_len: usize,
+            );
+        }
+    );
+}
+
+#[cfg(feature = "napi-3")]
+mod napi3 {
+    use super::super::types::*;
+
+    generate!(
+        extern "C" {
+            fn fatal_exception(env: Env, err: Value) -> Status;
         }
     );
 }
@@ -313,6 +331,8 @@ mod napi6 {
 }
 
 pub(crate) use napi1::*;
+#[cfg(feature = "napi-3")]
+pub(crate) use napi3::*;
 #[cfg(feature = "napi-4")]
 pub(crate) use napi4::*;
 #[cfg(feature = "napi-5")]
@@ -344,6 +364,9 @@ pub(crate) unsafe fn load(env: Env) -> Result<(), libloading::Error> {
 
     napi1::load(&host, version, 1)?;
 
+    #[cfg(feature = "napi-3")]
+    napi3::load(&host, version, 3)?;
+
     #[cfg(feature = "napi-4")]
     napi4::load(&host, version, 4)?;
 

crates/neon-runtime/src/napi/bindings/mod.rs

@@ -107,13 +107,13 @@ macro_rules! napi_name {
 /// ```
 macro_rules! generate {
     (extern "C" {
-        $(fn $name:ident($($param:ident: $ptype:ty$(,)?)*) -> $rtype:ty;)+
+        $(fn $name:ident($($param:ident: $ptype:ty$(,)?)*)$( -> $rtype:ty)?;)+
     }) => {
         pub(crate) struct Napi {
             $(
                 $name: unsafe extern "C" fn(
                     $($param: $ptype,)*
-                ) -> $rtype,
+                )$( -> $rtype)*,
             )*
         }
 
@@ -124,7 +124,7 @@ macro_rules! generate {
 
         static mut NAPI: Napi = {
             $(
-                unsafe extern "C" fn $name($(_: $ptype,)*) -> $rtype {
+                unsafe extern "C" fn $name($(_: $ptype,)*)$( -> $rtype)* {
                     panic_load()
                 }
             )*
@@ -159,7 +159,7 @@ macro_rules! generate {
 
         $(
             #[inline]
-            pub(crate) unsafe fn $name($($param: $ptype,)*) -> $rtype {
+            pub(crate) unsafe fn $name($($param: $ptype,)*)$( -> $rtype)* {
                 (NAPI.$name)($($param,)*)
             }
         )*

crates/neon-runtime/src/napi/error.rs

@@ -1,4 +1,5 @@
 use std::mem::MaybeUninit;
+use std::panic::Location;
 use std::ptr;
 
 use crate::napi::bindings as napi;
@@ -88,3 +89,17 @@ pub unsafe fn throw_error_from_utf8(env: Env, msg: *const u8, len: i32) {
 
     throw(env, err.assume_init());
 }
+
+#[track_caller]
+pub(super) unsafe fn fatal_error(message: &str) -> ! {
+    let location = Location::caller().to_string();
+
+    napi::fatal_error(
+        location.as_ptr().cast(),
+        location.len(),
+        message.as_ptr().cast(),
+        message.len(),
+    );
+
+    unreachable!("Expected napi_fatal_error to exit the process")
+}

crates/neon-runtime/src/napi/tsfn.rs

@@ -1,16 +1,25 @@
 //! Idiomatic Rust wrappers for N-API threadsafe functions
 
+use std::any::Any;
 use std::ffi::c_void;
 use std::mem::MaybeUninit;
+use std::panic::{catch_unwind, AssertUnwindSafe};
+use std::ptr;
 use std::sync::{Arc, Mutex};
 
 use crate::napi::bindings as napi;
-use crate::raw::Env;
+use crate::napi::error::fatal_error;
+use crate::raw::{Env, Local};
+
+type Panic = Box<dyn Any + Send + 'static>;
+
+const UNKNOWN_PANIC_MESSAGE: &str = "Unknown panic";
 
 #[derive(Debug)]
 struct Tsfn(napi::ThreadsafeFunction);
 
 unsafe impl Send for Tsfn {}
+
 unsafe impl Sync for Tsfn {}
 
 #[derive(Debug)]
@@ -138,7 +147,7 @@ impl<T: Send + 'static> ThreadsafeFunction<T> {
     /// Safety: `Env` must be valid for the current thread
     pub unsafe fn reference(&self, env: Env) {
         assert_eq!(
-            napi::ref_threadsafe_function(env, self.tsfn.0,),
+            napi::ref_threadsafe_function(env, self.tsfn.0),
             napi::Status::Ok,
         );
     }
@@ -147,7 +156,7 @@ impl<T: Send + 'static> ThreadsafeFunction<T> {
     /// Safety: `Env` must be valid for the current thread
     pub unsafe fn unref(&self, env: Env) {
         assert_eq!(
-            napi::unref_threadsafe_function(env, self.tsfn.0,),
+            napi::unref_threadsafe_function(env, self.tsfn.0),
             napi::Status::Ok,
         );
     }
@@ -161,6 +170,16 @@ impl<T: Send + 'static> ThreadsafeFunction<T> {
     }
 
     // Provides a C ABI wrapper for invoking the user supplied function pointer
+    // On panic or exception, creates an `uncaughtException` of the form:
+    // Error(msg: string) {
+    //     // Exception thrown
+    //     cause?: Error,
+    //     // Panic occurred
+    //     panic?: Error(msg: string) {
+    //         // Opaque panic type if it wasn't a string
+    //         cause?: JsBox<Panic>
+    //     }
+    // }
     unsafe extern "C" fn callback(
         env: Env,
         _js_callback: napi::Value,
@@ -169,19 +188,62 @@ impl<T: Send + 'static> ThreadsafeFunction<T> {
     ) {
         let Callback { callback, data } = *Box::from_raw(data as *mut Callback<T>);
 
-        // Event loop has terminated
+        // Event loop has terminated if `null`
         let env = if env.is_null() { None } else { Some(env) };
 
-        callback(env, data);
+        // Run the user supplied callback, catching panics
+        // This is unwind safe because control is never yielded back to the caller
+        let panic = catch_unwind(AssertUnwindSafe(move || callback(env, data))).err();
+
+        let env = if let Some(env) = env {
+            env
+        } else {
+            // If we don't have an Env, at most we can print a panic message
+            if let Some(panic) = panic {
+                let msg = no_panic::panic_msg(&panic).unwrap_or(UNKNOWN_PANIC_MESSAGE);
+
+                eprintln!("{}", msg);
+            }
+
+            return;
+        };
+
+        // Check and catch a thrown exception
+        let exception = no_panic::catch_exception(env);
+
+        // Create an error message or return if there wasn't a panic or exception
+        let msg = match (panic.is_some(), exception.is_some()) {
+            (true, true) => "A panic and exception occurred while executing a `neon::event::Channel::send` callback",
+            (true, false) => "A panic occurred while executing a `neon::event::Channel::send` callback",
+            (false, true) => "An exception occurred while executing a `neon::event::Channel::send` callback",
+            (false, false) => return
+        };
+
+        // Construct the `uncaughtException` Error object
+        let error = no_panic::error_from_message(env, msg);
+
+        // Add the exception to the error
+        if let Some(exception) = exception {
+            no_panic::set_property(env, error, "cause", exception);
+        };
+
+        // Add the panic to the error
+        if let Some(panic) = panic {
+            no_panic::set_property(env, error, "panic", no_panic::error_from_panic(env, panic));
+        }
+
+        // Throw an uncaught exception
+        if napi::fatal_exception(env, error) != napi::Status::Ok {
+            fatal_error("Failed to throw an uncaughtException");
+        }
     }
 }
 
 impl<T> Drop for ThreadsafeFunction<T> {
     fn drop(&mut self) {
         let is_finalized = self.is_finalized.lock().unwrap();
 
-        // tsfn was already finalized by `Environment::CleanupHandles()` in
-        // Node.js
+        // tsfn was already finalized by `Environment::CleanupHandles()` in Node.js
         if *is_finalized {
             return;
         }
@@ -194,3 +256,119 @@ impl<T> Drop for ThreadsafeFunction<T> {
         };
     }
 }
+
+// The following helpers do not panic and instead use `napi_fatal_error` to crash the
+// process in a controlled way, making them safe for use in FFI callbacks.
+mod no_panic {
+    use super::*;
+
+    #[track_caller]
+    pub(super) unsafe fn catch_exception(env: Env) -> Option<Local> {
+        if !is_exception_pending(env) {
+            return None;
+        }
+
+        let mut error = MaybeUninit::uninit();
+
+        if napi::get_and_clear_last_exception(env, error.as_mut_ptr()) != napi::Status::Ok {
+            fatal_error("Failed to get and clear the last exception");
+        }
+
+        Some(error.assume_init())
+    }
+
+    #[track_caller]
+    pub(super) unsafe fn error_from_message(env: Env, msg: &str) -> Local {
+        let msg = create_string(env, msg);
+        let mut err = MaybeUninit::uninit();
+
+        let status = napi::create_error(env, ptr::null_mut(), msg, err.as_mut_ptr());
+
+        let err = if status == napi::Status::Ok {
+            err.assume_init()
+        } else {
+            fatal_error("Failed to create an Error");
+        };
+
+        err
+    }
+
+    #[track_caller]
+    pub(super) unsafe fn error_from_panic(env: Env, panic: Panic) -> Local {
+        if let Some(msg) = panic_msg(&panic) {
+            error_from_message(env, msg)
+        } else {
+            let error = error_from_message(env, UNKNOWN_PANIC_MESSAGE);
+            let panic = external_from_panic(env, panic);
+
+            set_property(env, error, "cause", panic);
+            error
+        }
+    }
+
+    #[track_caller]
+    pub(super) unsafe fn set_property(env: Env, object: Local, key: &str, value: Local) {
+        let key = create_string(env, key);
+
+        if napi::set_property(env, object, key, value) != napi::Status::Ok {
+            fatal_error("Failed to set an object property");
+        }
+    }
+
+    #[track_caller]
+    pub(super) unsafe fn panic_msg(panic: &Panic) -> Option<&str> {
+        if let Some(msg) = panic.downcast_ref::<&str>() {
+            Some(msg)
+        } else if let Some(msg) = panic.downcast_ref::<String>() {
+            Some(&msg)
+        } else {
+            None
+        }
+    }
+
+    unsafe fn external_from_panic(env: Env, panic: Panic) -> Local {
+        let mut result = MaybeUninit::uninit();
+        let status = napi::create_external(
+            env,
+            Box::into_raw(Box::new(panic)).cast(),
+            Some(finalize_panic),
+            ptr::null_mut(),
+            result.as_mut_ptr(),
+        );
+
+        if status != napi::Status::Ok {
+            fatal_error("Failed to create a neon::types::JsBox from a panic");
+        }
+
+        result.assume_init()
+    }
+
+    extern "C" fn finalize_panic(_env: Env, data: *mut c_void, _hint: *mut c_void) {
+        unsafe {
+            Box::from_raw(data.cast::<Panic>());
+        }
+    }
+
+    #[track_caller]
+    unsafe fn create_string(env: Env, msg: &str) -> Local {
+        let mut string = MaybeUninit::uninit();
+        let status =
+            napi::create_string_utf8(env, msg.as_ptr().cast(), msg.len(), string.as_mut_ptr());
+
+        if status != napi::Status::Ok {
+            fatal_error("Failed to create a String");
+        }
+
+        string.assume_init()
+    }
+
+    unsafe fn is_exception_pending(env: Env) -> bool {
+        let mut throwing = false;
+
+        if napi::is_exception_pending(env, &mut throwing) != napi::Status::Ok {
+            fatal_error("Failed to check if an exception is pending");
+        }
+
+        throwing
+    }
+}