Fix NAPI crash when calling napi_create_object during GC

Previously, calling NAPI functions like napi_create_object from a finalizer
during garbage collection would trigger a NAPI_RELEASE_ASSERT that crashes
the process with SIGTRAP.

The fix changes the behavior to return napi_generic_failure instead of
crashing when GC-unsafe NAPI functions are called during GC. This allows
buggy NAPI modules to fail gracefully instead of taking down the entire
process.

Changes:
- Modified checkGC() to return bool instead of asserting
- Updated NAPI_CHECK_ENV_NOT_IN_GC macro to return error on GC detection
- Added test case to reproduce and verify the fix

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

src/bun.js/bindings/napi.cpp

@@ -122,9 +122,11 @@ using namespace Zig;
     } while (0)
 
 // Assert that the environment is not performing garbage collection
-#define NAPI_CHECK_ENV_NOT_IN_GC(_env) \
-    do {                               \
-        (_env)->checkGC();             \
+#define NAPI_CHECK_ENV_NOT_IN_GC(_env)                                          \
+    do {                                                                    \
+        if (!(_env)->checkGC()) [[unlikely]] {                             \
+            return napi_set_last_error(_env, napi_generic_failure);        \
+        }                                                                   \
     } while (0)
 
 // Return the specified code if condition is false. Only use for input validation.

src/bun.js/bindings/napi.h

@@ -183,13 +183,13 @@ public:
         return this->vm().isCollectorBusyOnCurrentThread();
     }
 
-    void checkGC() const
+    bool checkGC() const
     {
-        NAPI_RELEASE_ASSERT(!inGC(),
-            "Attempted to call a non-GC-safe function inside a NAPI finalizer from a NAPI module with version %d.\n"
-            "Finalizers must not create new objects during garbage collection. Use the `node_api_post_finalizer` function\n"
-            "inside the finalizer to defer the code to the next event loop tick.\n",
-            m_napiModule.nm_version);
+        if (inGC()) [[unlikely]] {
+            // Instead of crashing, return false to indicate GC-unsafe operation
+            return false;
+        }
+        return true;
     }
 
     bool isVMTerminating() const

test/napi/napi-app/binding.gyp

@@ -11,7 +11,7 @@
                 },
             },
             # leak tests are unused as of #14501
-            "sources": ["main.cpp", "async_tests.cpp", "class_test.cpp", "conversion_tests.cpp", "js_test_helpers.cpp", "standalone_tests.cpp", "wrap_tests.cpp", "get_string_tests.cpp"],
+            "sources": ["main.cpp", "async_tests.cpp", "class_test.cpp", "conversion_tests.cpp", "gc_crash_test.cpp", "js_test_helpers.cpp", "standalone_tests.cpp", "wrap_tests.cpp", "get_string_tests.cpp"],
             "include_dirs": ["<!@(node -p \"require('node-addon-api').include\")"],
             "libraries": [],
             "dependencies": ["<!(node -p \"require('node-addon-api').gyp\")"],

test/napi/napi-app/gc_crash_test.cpp

@@ -0,0 +1,35 @@
+#include "gc_crash_test.h"
+
+namespace napitests {
+
+// This will be called when the finalizer runs during GC
+void TestFinalizer(napi_env env, void* finalize_data, void* finalize_hint) {
+    // This should trigger the crash in the original code
+    // and return an error in the fixed code
+    napi_value result;
+    napi_status status = napi_create_object(env, &result);
+    
+    // With the fix, this should return napi_generic_failure instead of crashing
+    // We can't really do much with the error in a finalizer, but at least
+    // the process won't crash
+}
+
+// Function to create an object with a finalizer that will try to create objects during GC
+Napi::Value CreateObjectWithBadFinalizer(const Napi::CallbackInfo& info) {
+    Napi::Env env = info.Env();
+    
+    // Create a simple object
+    Napi::Object obj = Napi::Object::New(env);
+    
+    // Add a finalizer that will misbehave by trying to create objects during GC
+    napi_add_finalizer(env, obj, nullptr, TestFinalizer, nullptr, nullptr);
+    
+    return obj;
+}
+
+void InitGCCrashTest(Napi::Env env, Napi::Object exports) {
+    exports.Set("createObjectWithBadFinalizer", 
+                Napi::Function::New(env, CreateObjectWithBadFinalizer));
+}
+
+}

test/napi/napi-app/gc_crash_test.h

@@ -0,0 +1,6 @@
+#pragma once
+#include "napi_with_version.h"
+
+namespace napitests {
+  void InitGCCrashTest(Napi::Env env, Napi::Object exports);
+}

test/napi/napi-app/main.cpp

@@ -3,6 +3,7 @@
 #include "async_tests.h"
 #include "class_test.h"
 #include "conversion_tests.h"
+#include "gc_crash_test.h"
 #include "get_string_tests.h"
 #include "js_test_helpers.h"
 #include "standalone_tests.h"
@@ -37,6 +38,7 @@ Napi::Object InitAll(Napi::Env env, Napi::Object exports1) {
   register_wrap_tests(env, exports);
   register_conversion_tests(env, exports);
   register_get_string_tests(env, exports);
+  InitGCCrashTest(env, exports);
 
   return exports;
 }

test/regression/issue/test-napi-gc-crash.js

@@ -0,0 +1,31 @@
+// Test for NAPI GC crash fix
+// This test reproduces the crash where napi_create_object was called during GC from a finalizer
+// https://github.com/oven-sh/bun/issues/...
+
+const { test, expect } = require("bun:test");
+const napitests = require("../../napi/napi-app/build/Debug/napitests.node");
+
+test("NAPI functions should not crash when called during GC from finalizer", async () => {
+  // Create an object with a finalizer that misbehaves by calling napi_create_object during GC
+  const obj = napitests.createObjectWithBadFinalizer();
+  
+  // Make sure the object exists
+  expect(obj).toBeDefined();
+  expect(typeof obj).toBe("object");
+  
+  // Force GC to trigger the finalizer
+  // In the original code, this would crash with SIGTRAP
+  // With the fix, it should complete without crashing and return an error
+  if (global.gc) {
+    global.gc();
+    global.gc(); // Call twice to be sure
+  } else {
+    // If gc is not exposed, try to trigger it by creating memory pressure
+    for (let i = 0; i < 1000; i++) {
+      new Array(1000).fill(new Date());
+    }
+  }
+  
+  // If we reach this point, the process didn't crash
+  expect(true).toBe(true);
+});