fix: use weak references for stored threadsafe functions (#5025)

Co-authored-by: Franco Victorio <[email protected]>

Author: Wodann

.changeset/ten-spoons-study.md

@@ -0,0 +1,5 @@
+---
+"@nomicfoundation/edr": patch
+---
+
+Fix node.js runtime freezing on shutdown

Cargo.lock

@@ -14,8 +14,8 @@ k256 = { version = "0.13.1", default-features = false, features = ["arithmetic",
 log = { version = "0.4.20", default-features = false }
 # when napi is pinned, be sure to pin napi-derive to the same version
 # The `async` feature ensures that a tokio runtime is available
-napi = { version = "2.12.4", default-features = false, features = ["async", "error_anyhow", "napi8", "serde-json"] }
-napi-derive = "2.12.3"
+napi = { version = "2.16.0", default-features = false, features = ["async", "error_anyhow", "napi8", "serde-json"] }
+napi-derive = "2.16.0"
 edr_defaults = { version = "0.2.0-dev", path = "../edr_defaults" }
 edr_evm = { version = "0.2.0-dev", path = "../edr_evm", features = ["tracing"]}
 edr_eth = { version = "0.2.0-dev", path = "../edr_eth" }

crates/edr_napi/Cargo.toml

@@ -52,8 +52,8 @@
     "universal": "napi universal",
     "version": "napi version",
     "pretest": "pnpm build",
-    "test": "pnpm tsc && mocha --recursive \"test/**/*.ts\" --exit",
-    "testNoBuild": "pnpm tsc && mocha --recursive \"test/**/*.ts\" --exit",
+    "test": "pnpm tsc && mocha --recursive \"test/**/*.ts\"",
+    "testNoBuild": "pnpm tsc && mocha --recursive \"test/**/*.ts\"",
     "clean": "rm -rf @nomicfoundation/edr.node"
   }
 }

crates/edr_napi/package.json

@@ -1,14 +1,16 @@
-use std::sync::mpsc::{channel, Sender};
+use std::sync::mpsc::channel;
 
 use edr_eth::{Address, Bytes};
-use napi::{bindgen_prelude::Buffer, Env, JsFunction, NapiRaw, Status};
+use napi::{
+    bindgen_prelude::Buffer,
+    threadsafe_function::{
+        ErrorStrategy, ThreadSafeCallContext, ThreadsafeFunction, ThreadsafeFunctionCallMode,
+    },
+    Env, JsFunction, Status,
+};
 use napi_derive::napi;
 
-use crate::{
-    cast::TryCast,
-    sync::{await_promise, handle_error},
-    threadsafe_function::{ThreadSafeCallContext, ThreadsafeFunction, ThreadsafeFunctionCallMode},
-};
+use crate::cast::TryCast;
 
 /// The result of executing a call override.
 #[napi(object)]
@@ -34,20 +36,16 @@ impl TryCast<Option<edr_provider::CallOverrideResult>> for Option<CallOverrideRe
 struct CallOverrideCall {
     contract_address: Address,
     data: Bytes,
-    sender: Sender<napi::Result<Option<edr_provider::CallOverrideResult>>>,
 }
 
 #[derive(Clone)]
 pub struct CallOverrideCallback {
-    call_override_callback_fn: ThreadsafeFunction<CallOverrideCall>,
+    call_override_callback_fn: ThreadsafeFunction<CallOverrideCall, ErrorStrategy::Fatal>,
 }
 
 impl CallOverrideCallback {
     pub fn new(env: &Env, call_override_callback: JsFunction) -> napi::Result<Self> {
-        let call_override_callback_fn = ThreadsafeFunction::create(
-            env.raw(),
-            // SAFETY: The callback is guaranteed to be valid for the lifetime of the inspector.
-            unsafe { call_override_callback.raw() },
+        let mut call_override_callback_fn = call_override_callback.create_threadsafe_function(
             0,
             |ctx: ThreadSafeCallContext<CallOverrideCall>| {
                 let address = ctx
@@ -60,17 +58,14 @@ impl CallOverrideCallback {
                     .create_buffer_with_data(ctx.value.data.to_vec())?
                     .into_raw();
 
-                let sender = ctx.value.sender.clone();
-                let promise = ctx.callback.call(None, &[address, data])?;
-                let result = await_promise::<
-                    Option<CallOverrideResult>,
-                    Option<edr_provider::CallOverrideResult>,
-                >(ctx.env, promise, ctx.value.sender);
-
-                handle_error(sender, result)
+                Ok(vec![address, data])
             },
         )?;
 
+        // Maintain a weak reference to the function to avoid the event loop from
+        // exiting.
+        call_override_callback_fn.unref(env)?;
+
         Ok(Self {
             call_override_callback_fn,
         })
@@ -83,13 +78,22 @@ impl CallOverrideCallback {
     ) -> Option<edr_provider::CallOverrideResult> {
         let (sender, receiver) = channel();
 
-        let status = self.call_override_callback_fn.call(
+        let status = self.call_override_callback_fn.call_with_return_value(
             CallOverrideCall {
                 contract_address,
                 data,
-                sender,
             },
             ThreadsafeFunctionCallMode::Blocking,
+            move |result: Option<CallOverrideResult>| {
+                let result = result.try_cast();
+
+                sender.send(result).map_err(|_error| {
+                    napi::Error::new(
+                        Status::GenericFailure,
+                        "Failed to send result from call_override_callback",
+                    )
+                })
+            },
         );
 
         assert_eq!(status, Status::Ok, "Call override callback failed");

crates/edr_napi/src/call_override.rs

@@ -16,7 +16,5 @@ mod result;
 #[cfg(feature = "scenarios")]
 mod scenarios;
 mod subscribe;
-mod sync;
-mod threadsafe_function;
 mod trace;
 mod withdrawal;

crates/edr_napi/src/lib.rs

@@ -1,5 +1,3 @@
-use std::mem;
-
 use napi::{bindgen_prelude::Buffer, Env, JsBuffer, JsBufferValue};
 use napi_derive::napi;
 
@@ -19,18 +17,9 @@ impl ExecutionLog {
             .map(|topic| Buffer::from(topic.as_slice()))
             .collect();
 
-        let data = log.data.data.clone();
-        let data = unsafe {
-            env.create_buffer_with_borrowed_data(
-                data.as_ptr(),
-                data.len(),
-                data,
-                |data: edr_eth::Bytes, _env| {
-                    mem::drop(data);
-                },
-            )
-        }
-        .map(JsBufferValue::into_raw)?;
+        let data = env
+            .create_buffer_with_data(log.data.data.to_vec())
+            .map(JsBufferValue::into_raw)?;
 
         Ok(Self {
             address: Buffer::from(log.address.as_slice()),