Avoid creating a Napi handle scope within a finalizer

src/bun.js/bindings/napi.h

@@ -39,7 +39,9 @@ static inline Zig::GlobalObject* toJS(napi_env val)
 static inline napi_value toNapi(JSC::JSValue val, Zig::GlobalObject* globalObject)
 {
     if (val.isCell()) {
-        globalObject->m_currentNapiHandleScopeImpl.get()->append(val);
+        if (auto* scope = globalObject->m_currentNapiHandleScopeImpl.get()) {
+            scope->append(val);
+        }
     }
     return reinterpret_cast<napi_value>(JSC::JSValue::encode(val));
 }

src/bun.js/bindings/napi_handle_scope.cpp

@@ -80,11 +80,24 @@ NapiHandleScopeImpl::Slot* NapiHandleScopeImpl::reserveSlot()
 
 NapiHandleScopeImpl* NapiHandleScope::push(Zig::GlobalObject* globalObject, bool escapable)
 {
-    auto* impl = NapiHandleScopeImpl::create(globalObject->vm(),
+    auto& vm = globalObject->vm();
+    // Do not create a new handle scope while a finalizer is in progress
+    // This state is possible because we call napi finalizers immediately
+    // so a finalizer can be called while an allocation is in progress.
+    // An example where this happens:
+    // 1. Use the `sqlite3` package
+    // 2. Do an allocation in a hot code path
+    // 3. the napi_ref finalizer is called while the constructor is running
+    // 4. The finalizer creates a new handle scope (yes, it should not do that. No, we can't change that.)
+    if (vm.heap.mutatorState() == JSC::MutatorState::Sweeping) {
+        return nullptr;
+    }
+
+    auto* impl = NapiHandleScopeImpl::create(vm,
         globalObject->NapiHandleScopeImplStructure(),
         globalObject->m_currentNapiHandleScopeImpl.get(),
         escapable);
-    globalObject->m_currentNapiHandleScopeImpl.set(globalObject->vm(), globalObject, impl);
+    globalObject->m_currentNapiHandleScopeImpl.set(vm, globalObject, impl);
     return impl;
 }
 

src/napi/napi.zig

@@ -70,7 +70,7 @@ pub const NapiHandleScope = opaque {
     extern fn NapiHandleScope__append(globalObject: *JSC.JSGlobalObject, value: JSC.JSValueReprInt) void;
     extern fn NapiHandleScope__escape(handleScope: *NapiHandleScope, value: JSC.JSValueReprInt) bool;
 
-    pub fn push(env: napi_env, escapable: bool) *NapiHandleScope {
+    pub fn push(env: napi_env, escapable: bool) ?*NapiHandleScope {
         return NapiHandleScope__push(env, escapable);
     }
 
@@ -744,7 +744,7 @@ pub extern fn napi_reference_unref(env: napi_env, ref: *Ref, result: [*c]u32) na
 pub extern fn napi_get_reference_value(env: napi_env, ref: *Ref, result: *napi_value) napi_status;
 pub extern fn napi_get_reference_value_internal(ref: *Ref) JSC.JSValue;
 
-pub export fn napi_open_handle_scope(env: napi_env, result_: ?*napi_handle_scope) napi_status {
+pub export fn napi_open_handle_scope(env: napi_env, result_: ?*?napi_handle_scope) napi_status {
     log("napi_open_handle_scope", .{});
     const result = result_ orelse {
         return invalidArg();
@@ -753,9 +753,12 @@ pub export fn napi_open_handle_scope(env: napi_env, result_: ?*napi_handle_scope
     return .ok;
 }
 
-pub export fn napi_close_handle_scope(env: napi_env, handle_scope: napi_handle_scope) napi_status {
+pub export fn napi_close_handle_scope(env: napi_env, handle_scope: ?napi_handle_scope) napi_status {
     log("napi_close_handle_scope", .{});
-    handle_scope.pop(env);
+    if (handle_scope) |scope| {
+        scope.pop(env);
+    }
+
     return .ok;
 }
 
@@ -821,17 +824,19 @@ fn notImplementedYet(comptime name: []const u8) void {
     );
 }
 
-pub export fn napi_open_escapable_handle_scope(env: napi_env, result_: ?*napi_escapable_handle_scope) napi_status {
+pub export fn napi_open_escapable_handle_scope(env: napi_env, result_: ?*?napi_escapable_handle_scope) napi_status {
     log("napi_open_escapable_handle_scope", .{});
     const result = result_ orelse {
         return invalidArg();
     };
     result.* = NapiHandleScope.push(env, true);
     return .ok;
 }
-pub export fn napi_close_escapable_handle_scope(env: napi_env, scope: napi_escapable_handle_scope) napi_status {
+pub export fn napi_close_escapable_handle_scope(env: napi_env, scope: ?napi_escapable_handle_scope) napi_status {
     log("napi_close_escapable_handle_scope", .{});
-    scope.pop(env);
+    if (scope) |s| {
+        s.pop(env);
+    }
     return .ok;
 }
 pub export fn napi_escape_handle(_: napi_env, scope: napi_escapable_handle_scope, escapee: napi_value, result_: ?*napi_value) napi_status {
@@ -1195,7 +1200,7 @@ pub const napi_async_work = struct {
 
     pub fn runFromJS(this: *napi_async_work) void {
         const handle_scope = NapiHandleScope.push(this.global, false);
-        defer handle_scope.pop(this.global);
+        defer if (handle_scope) |scope| scope.pop(this.global);
         this.complete.?(
             this.global,
             if (this.status.load(.seq_cst) == @intFromEnum(Status.cancelled))
@@ -1577,7 +1582,7 @@ pub const ThreadSafeFunction = struct {
                 }
 
                 const handle_scope = NapiHandleScope.push(globalObject, false);
-                defer handle_scope.pop(globalObject);
+                defer if (handle_scope) |scope| scope.pop(globalObject);
                 cb.napi_threadsafe_function_call_js(globalObject, napi_value.create(globalObject, cb.js), this.ctx, task);
             },
         }