mirror of
https://github.com/oven-sh/bun
synced 2026-02-09 10:28:47 +00:00
## Summary Fixes a critical segmentation fault crash occurring during NAPI finalizer cleanup when finalizers trigger GC operations. This crash was reported with `node-sqlite3` and other NAPI modules during process exit. ## Root Cause The crash was caused by **iterator invalidation** in `napi_env__::cleanup()`: 1. Range-based for loop iterates over `m_finalizers` (std::unordered_set) 2. `boundFinalizer.call(this)` executes finalizer callbacks 3. Finalizers can trigger JavaScript execution and GC operations 4. GC can add/remove entries from `m_finalizers` during iteration 5. **Iterator invalidation** → undefined behavior → segfault ## Solution Added `JSC::DeferGCForAWhile deferGC(m_vm)` scope during entire finalizer cleanup to prevent any GC operations from occurring during iteration. ### Why This Approach? - **Addresses root cause**: Prevents GC entirely during critical section - **Simple & safe**: One-line RAII fix using existing JSC patterns - **Minimal impact**: Only affects process cleanup (not runtime performance) - **Proven pattern**: Already used elsewhere in Bun's codebase - **Better than alternatives**: Cleaner than Node.js manual iterator approach ## Testing Added comprehensive test (`test_finalizer_iterator_invalidation.c`) that reproduces the crash by: - Creating finalizers that trigger GC operations - Forcing JavaScript execution during finalization - Allocating objects that can trigger more GC - Calling process exit to trigger finalizer cleanup **Before fix**: Segmentation fault **After fix**: Clean exit ✅ ## Files Changed - `src/bun.js/bindings/napi.h`: Core fix + include - `test/napi/napi-app/test_finalizer_iterator_invalidation.c`: Test addon - `test/napi/napi-app/binding.gyp`: Build config for test addon - `test/regression/issue/napi-finalizer-crash.test.ts`: Integration test ## Test Plan - [x] Reproduces original crash without fix - [x] Passes cleanly with fix applied - [x] Existing NAPI tests continue to pass - [x] Manual testing with node-sqlite3 scenarios 🤖 Generated with [Claude Code](https://claude.ai/code) --------- Co-authored-by: Claude Bot <claude-bot@bun.sh> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Kai Tamkun <kai@tamkun.io>
123 lines
4.3 KiB
C
123 lines
4.3 KiB
C
#include <node_api.h>
|
|
#include <stdio.h>
|
|
#include <stdlib.h>
|
|
#include <assert.h>
|
|
|
|
static int finalize_call_count = 0;
|
|
static napi_env saved_env = NULL;
|
|
|
|
// This finalizer will trigger operations that can cause GC
|
|
static void problematic_finalizer(napi_env env, void* finalize_data, void* finalize_hint) {
|
|
finalize_call_count++;
|
|
printf("Finalizer %d called\n", finalize_call_count);
|
|
|
|
// Save env for later use
|
|
if (!saved_env) saved_env = env;
|
|
|
|
// Operations that can trigger GC and modify m_finalizers during iteration:
|
|
|
|
// 1. Try to force GC if available
|
|
napi_value global, gc_func;
|
|
if (napi_get_global(env, &global) == napi_ok) {
|
|
if (napi_get_named_property(env, global, "gc", &gc_func) == napi_ok) {
|
|
napi_valuetype type;
|
|
if (napi_typeof(env, gc_func, &type) == napi_ok && type == napi_function) {
|
|
napi_value result;
|
|
napi_call_function(env, global, gc_func, 0, NULL, &result);
|
|
printf(" - GC triggered from finalizer %d\n", finalize_call_count);
|
|
}
|
|
}
|
|
}
|
|
|
|
// 2. Create and immediately abandon objects (can trigger GC)
|
|
for (int i = 0; i < 10; i++) {
|
|
napi_value obj;
|
|
napi_create_object(env, &obj);
|
|
napi_value arr;
|
|
napi_create_array_with_length(env, 100, &arr);
|
|
}
|
|
|
|
// 3. Try to run some JavaScript that might trigger GC
|
|
napi_value code_string, result;
|
|
const char* js_code = "Array.from({length: 100}, (_, i) => ({id: i, data: new Array(100).fill(i)}))";
|
|
if (napi_create_string_utf8(env, js_code, NAPI_AUTO_LENGTH, &code_string) == napi_ok) {
|
|
// This might trigger more allocations and GC
|
|
napi_run_script(env, code_string, &result);
|
|
}
|
|
|
|
printf(" - Finalizer %d completed\n", finalize_call_count);
|
|
}
|
|
|
|
static napi_value create_objects_with_problematic_finalizers(napi_env env, napi_callback_info info) {
|
|
size_t argc = 1;
|
|
napi_value args[1];
|
|
napi_get_cb_info(env, info, &argc, args, NULL, NULL);
|
|
|
|
int count = 10; // default
|
|
if (argc >= 1) {
|
|
napi_get_value_int32(env, args[0], &count);
|
|
}
|
|
|
|
printf("Creating %d objects with problematic finalizers\n", count);
|
|
|
|
napi_value result_array;
|
|
napi_create_array_with_length(env, count, &result_array);
|
|
|
|
for (int i = 0; i < count; i++) {
|
|
napi_value obj;
|
|
napi_create_object(env, &obj);
|
|
|
|
// Allocate some data for the finalizer
|
|
int* data = (int*)malloc(sizeof(int));
|
|
*data = i;
|
|
|
|
// Wrap object with the problematic finalizer
|
|
napi_wrap(env, obj, data, problematic_finalizer, NULL, NULL);
|
|
|
|
napi_set_element(env, result_array, i, obj);
|
|
}
|
|
|
|
return result_array;
|
|
}
|
|
|
|
static napi_value get_finalize_count(napi_env env, napi_callback_info info) {
|
|
napi_value result;
|
|
napi_create_int32(env, finalize_call_count, &result);
|
|
return result;
|
|
}
|
|
|
|
static napi_value force_cleanup_and_exit(napi_env env, napi_callback_info info) {
|
|
printf("Forcing cleanup and exit - this would crash before the fix\n");
|
|
|
|
// Try to trigger GC first
|
|
napi_value global, gc_func;
|
|
if (napi_get_global(env, &global) == napi_ok) {
|
|
if (napi_get_named_property(env, global, "gc", &gc_func) == napi_ok) {
|
|
napi_valuetype type;
|
|
if (napi_typeof(env, gc_func, &type) == napi_ok && type == napi_function) {
|
|
napi_value result;
|
|
napi_call_function(env, global, gc_func, 0, NULL, &result);
|
|
printf("GC triggered before exit\n");
|
|
}
|
|
}
|
|
}
|
|
|
|
// This will cause process exit and trigger the finalizer cleanup
|
|
// where the crash would occur due to iterator invalidation
|
|
exit(0);
|
|
}
|
|
|
|
static napi_value init(napi_env env, napi_value exports) {
|
|
napi_property_descriptor properties[] = {
|
|
{ "createProblematicObjects", 0, create_objects_with_problematic_finalizers, 0, 0, 0, napi_default, 0 },
|
|
{ "getFinalizeCount", 0, get_finalize_count, 0, 0, 0, napi_default, 0 },
|
|
{ "forceCleanupAndExit", 0, force_cleanup_and_exit, 0, 0, 0, napi_default, 0 }
|
|
};
|
|
|
|
size_t property_count = sizeof(properties) / sizeof(properties[0]);
|
|
napi_define_properties(env, exports, property_count, properties);
|
|
|
|
return exports;
|
|
}
|
|
|
|
NAPI_MODULE(NODE_GYP_MODULE_NAME, init) |