From f37df906b46ff549d4b0833ab144f47b44b922a2 Mon Sep 17 00:00:00 2001 From: Kai Tamkun Date: Thu, 14 Nov 2024 13:28:52 -0800 Subject: [PATCH 1/9] Add typedef for node_api_basic_env in napi tests --- .../test/js-native-api/common.h | 102 +++++++++--------- 1 file changed, 50 insertions(+), 52 deletions(-) diff --git a/test/napi/node-napi-tests/test/js-native-api/common.h b/test/napi/node-napi-tests/test/js-native-api/common.h index 49cdc066ea..e215a56735 100644 --- a/test/napi/node-napi-tests/test/js-native-api/common.h +++ b/test/napi/node-napi-tests/test/js-native-api/common.h @@ -2,49 +2,48 @@ #define JS_NATIVE_API_COMMON_H_ #include -#include // abort() +#include // abort() + +#include // Empty value so that macros here are able to return NULL or void -#define NODE_API_RETVAL_NOTHING // Intentionally blank #define +#define NODE_API_RETVAL_NOTHING // Intentionally blank #define -#define GET_AND_THROW_LAST_ERROR(env) \ - do { \ - const napi_extended_error_info *error_info; \ - napi_get_last_error_info((env), &error_info); \ - bool is_pending; \ - const char* err_message = error_info->error_message; \ - napi_is_exception_pending((env), &is_pending); \ - /* If an exception is already pending, don't rethrow it */ \ - if (!is_pending) { \ - const char* error_message = err_message != NULL ? \ - err_message : \ - "empty error message"; \ - napi_throw_error((env), NULL, error_message); \ - } \ +#define GET_AND_THROW_LAST_ERROR(env) \ + do { \ + const napi_extended_error_info *error_info; \ + napi_get_last_error_info((env), &error_info); \ + bool is_pending; \ + const char *err_message = error_info->error_message; \ + napi_is_exception_pending((env), &is_pending); \ + /* If an exception is already pending, don't rethrow it */ \ + if (!is_pending) { \ + const char *error_message = \ + err_message != NULL ? err_message : "empty error message"; \ + napi_throw_error((env), NULL, error_message); \ + } \ } while (0) // The basic version of GET_AND_THROW_LAST_ERROR. We cannot access any // exceptions and we cannot fail by way of JS exception, so we abort. #define FATALLY_FAIL_WITH_LAST_ERROR(env) \ do { \ - const napi_extended_error_info* error_info; \ + const napi_extended_error_info *error_info; \ napi_get_last_error_info((env), &error_info); \ - const char* err_message = error_info->error_message; \ - const char* error_message = \ + const char *err_message = error_info->error_message; \ + const char *error_message = \ err_message != NULL ? err_message : "empty error message"; \ fprintf(stderr, "%s\n", error_message); \ abort(); \ } while (0) -#define NODE_API_ASSERT_BASE(env, assertion, message, ret_val) \ - do { \ - if (!(assertion)) { \ - napi_throw_error( \ - (env), \ - NULL, \ - "assertion (" #assertion ") failed: " message); \ - return ret_val; \ - } \ +#define NODE_API_ASSERT_BASE(env, assertion, message, ret_val) \ + do { \ + if (!(assertion)) { \ + napi_throw_error((env), NULL, \ + "assertion (" #assertion ") failed: " message); \ + return ret_val; \ + } \ } while (0) #define NODE_API_BASIC_ASSERT_BASE(assertion, message, ret_val) \ @@ -58,23 +57,23 @@ // Returns NULL on failed assertion. // This is meant to be used inside napi_callback methods. -#define NODE_API_ASSERT(env, assertion, message) \ +#define NODE_API_ASSERT(env, assertion, message) \ NODE_API_ASSERT_BASE(env, assertion, message, NULL) // Returns empty on failed assertion. // This is meant to be used inside functions with void return type. -#define NODE_API_ASSERT_RETURN_VOID(env, assertion, message) \ +#define NODE_API_ASSERT_RETURN_VOID(env, assertion, message) \ NODE_API_ASSERT_BASE(env, assertion, message, NODE_API_RETVAL_NOTHING) #define NODE_API_BASIC_ASSERT_RETURN_VOID(assertion, message) \ NODE_API_BASIC_ASSERT_BASE(assertion, message, NODE_API_RETVAL_NOTHING) -#define NODE_API_CALL_BASE(env, the_call, ret_val) \ - do { \ - if ((the_call) != napi_ok) { \ - GET_AND_THROW_LAST_ERROR((env)); \ - return ret_val; \ - } \ +#define NODE_API_CALL_BASE(env, the_call, ret_val) \ + do { \ + if ((the_call) != napi_ok) { \ + GET_AND_THROW_LAST_ERROR((env)); \ + return ret_val; \ + } \ } while (0) #define NODE_API_BASIC_CALL_BASE(env, the_call, ret_val) \ @@ -86,17 +85,16 @@ } while (0) // Returns NULL if the_call doesn't return napi_ok. -#define NODE_API_CALL(env, the_call) \ - NODE_API_CALL_BASE(env, the_call, NULL) +#define NODE_API_CALL(env, the_call) NODE_API_CALL_BASE(env, the_call, NULL) // Returns empty if the_call doesn't return napi_ok. -#define NODE_API_CALL_RETURN_VOID(env, the_call) \ +#define NODE_API_CALL_RETURN_VOID(env, the_call) \ NODE_API_CALL_BASE(env, the_call, NODE_API_RETVAL_NOTHING) #define NODE_API_BASIC_CALL_RETURN_VOID(env, the_call) \ NODE_API_BASIC_CALL_BASE(env, the_call, NODE_API_RETVAL_NOTHING) -#define NODE_API_CHECK_STATUS(the_call) \ +#define NODE_API_CHECK_STATUS(the_call) \ do { \ napi_status status = (the_call); \ if (status != napi_ok) { \ @@ -107,26 +105,26 @@ #define NODE_API_ASSERT_STATUS(env, assertion, message) \ NODE_API_ASSERT_BASE(env, assertion, message, napi_generic_failure) -#define DECLARE_NODE_API_PROPERTY(name, func) \ - { (name), NULL, (func), NULL, NULL, NULL, napi_default, NULL } +#define DECLARE_NODE_API_PROPERTY(name, func) \ + {(name), NULL, (func), NULL, NULL, NULL, napi_default, NULL} -#define DECLARE_NODE_API_GETTER(name, func) \ - { (name), NULL, NULL, (func), NULL, NULL, napi_default, NULL } +#define DECLARE_NODE_API_GETTER(name, func) \ + {(name), NULL, NULL, (func), NULL, NULL, napi_default, NULL} -#define DECLARE_NODE_API_PROPERTY_VALUE(name, value) \ - { (name), NULL, NULL, NULL, NULL, (value), napi_default, NULL } +#define DECLARE_NODE_API_PROPERTY_VALUE(name, value) \ + {(name), NULL, NULL, NULL, NULL, (value), napi_default, NULL} -static inline void add_returned_status(napi_env env, - const char* key, +static inline void add_returned_status(napi_env env, const char *key, napi_value object, - char* expected_message, + char *expected_message, napi_status expected_status, napi_status actual_status); -static inline void add_last_status(napi_env env, - const char* key, +static inline void add_last_status(napi_env env, const char *key, napi_value return_value); #include "common-inl.h" -#endif // JS_NATIVE_API_COMMON_H_ +typedef node_api_nogc_env node_api_basic_env; + +#endif // JS_NATIVE_API_COMMON_H_ From 134f66c24dc002a99f2e2313eb70f8365c97c246 Mon Sep 17 00:00:00 2001 From: Kai Tamkun Date: Thu, 14 Nov 2024 13:32:08 -0800 Subject: [PATCH 2/9] Undo accidental formatting --- .../test/js-native-api/common.h | 98 ++++++++++--------- 1 file changed, 52 insertions(+), 46 deletions(-) diff --git a/test/napi/node-napi-tests/test/js-native-api/common.h b/test/napi/node-napi-tests/test/js-native-api/common.h index e215a56735..b4ffa154eb 100644 --- a/test/napi/node-napi-tests/test/js-native-api/common.h +++ b/test/napi/node-napi-tests/test/js-native-api/common.h @@ -2,48 +2,51 @@ #define JS_NATIVE_API_COMMON_H_ #include -#include // abort() +#include // abort() #include // Empty value so that macros here are able to return NULL or void -#define NODE_API_RETVAL_NOTHING // Intentionally blank #define +#define NODE_API_RETVAL_NOTHING // Intentionally blank #define -#define GET_AND_THROW_LAST_ERROR(env) \ - do { \ - const napi_extended_error_info *error_info; \ - napi_get_last_error_info((env), &error_info); \ - bool is_pending; \ - const char *err_message = error_info->error_message; \ - napi_is_exception_pending((env), &is_pending); \ - /* If an exception is already pending, don't rethrow it */ \ - if (!is_pending) { \ - const char *error_message = \ - err_message != NULL ? err_message : "empty error message"; \ - napi_throw_error((env), NULL, error_message); \ - } \ +#define GET_AND_THROW_LAST_ERROR(env) \ + do { \ + const napi_extended_error_info *error_info; \ + napi_get_last_error_info((env), &error_info); \ + bool is_pending; \ + const char* err_message = error_info->error_message; \ + napi_is_exception_pending((env), &is_pending); \ + /* If an exception is already pending, don't rethrow it */ \ + if (!is_pending) { \ + const char* error_message = err_message != NULL ? \ + err_message : \ + "empty error message"; \ + napi_throw_error((env), NULL, error_message); \ + } \ } while (0) // The basic version of GET_AND_THROW_LAST_ERROR. We cannot access any // exceptions and we cannot fail by way of JS exception, so we abort. #define FATALLY_FAIL_WITH_LAST_ERROR(env) \ do { \ - const napi_extended_error_info *error_info; \ + const napi_extended_error_info* error_info; \ napi_get_last_error_info((env), &error_info); \ - const char *err_message = error_info->error_message; \ - const char *error_message = \ + const char* err_message = error_info->error_message; \ + const char* error_message = \ err_message != NULL ? err_message : "empty error message"; \ fprintf(stderr, "%s\n", error_message); \ abort(); \ } while (0) -#define NODE_API_ASSERT_BASE(env, assertion, message, ret_val) \ - do { \ - if (!(assertion)) { \ - napi_throw_error((env), NULL, \ - "assertion (" #assertion ") failed: " message); \ - return ret_val; \ - } \ +#define NODE_API_ASSERT_BASE(env, assertion, message, ret_val) \ + do { \ + if (!(assertion)) { \ + napi_throw_error( \ + (env), \ + NULL, \ + "assertion (" #assertion ") failed: " message); \ + return ret_val; \ + } \ } while (0) #define NODE_API_BASIC_ASSERT_BASE(assertion, message, ret_val) \ @@ -57,23 +60,23 @@ // Returns NULL on failed assertion. // This is meant to be used inside napi_callback methods. -#define NODE_API_ASSERT(env, assertion, message) \ +#define NODE_API_ASSERT(env, assertion, message) \ NODE_API_ASSERT_BASE(env, assertion, message, NULL) // Returns empty on failed assertion. // This is meant to be used inside functions with void return type. -#define NODE_API_ASSERT_RETURN_VOID(env, assertion, message) \ +#define NODE_API_ASSERT_RETURN_VOID(env, assertion, message) \ NODE_API_ASSERT_BASE(env, assertion, message, NODE_API_RETVAL_NOTHING) #define NODE_API_BASIC_ASSERT_RETURN_VOID(assertion, message) \ NODE_API_BASIC_ASSERT_BASE(assertion, message, NODE_API_RETVAL_NOTHING) -#define NODE_API_CALL_BASE(env, the_call, ret_val) \ - do { \ - if ((the_call) != napi_ok) { \ - GET_AND_THROW_LAST_ERROR((env)); \ - return ret_val; \ - } \ +#define NODE_API_CALL_BASE(env, the_call, ret_val) \ + do { \ + if ((the_call) != napi_ok) { \ + GET_AND_THROW_LAST_ERROR((env)); \ + return ret_val; \ + } \ } while (0) #define NODE_API_BASIC_CALL_BASE(env, the_call, ret_val) \ @@ -85,16 +88,17 @@ } while (0) // Returns NULL if the_call doesn't return napi_ok. -#define NODE_API_CALL(env, the_call) NODE_API_CALL_BASE(env, the_call, NULL) +#define NODE_API_CALL(env, the_call) \ + NODE_API_CALL_BASE(env, the_call, NULL) // Returns empty if the_call doesn't return napi_ok. -#define NODE_API_CALL_RETURN_VOID(env, the_call) \ +#define NODE_API_CALL_RETURN_VOID(env, the_call) \ NODE_API_CALL_BASE(env, the_call, NODE_API_RETVAL_NOTHING) #define NODE_API_BASIC_CALL_RETURN_VOID(env, the_call) \ NODE_API_BASIC_CALL_BASE(env, the_call, NODE_API_RETVAL_NOTHING) -#define NODE_API_CHECK_STATUS(the_call) \ +#define NODE_API_CHECK_STATUS(the_call) \ do { \ napi_status status = (the_call); \ if (status != napi_ok) { \ @@ -105,26 +109,28 @@ #define NODE_API_ASSERT_STATUS(env, assertion, message) \ NODE_API_ASSERT_BASE(env, assertion, message, napi_generic_failure) -#define DECLARE_NODE_API_PROPERTY(name, func) \ - {(name), NULL, (func), NULL, NULL, NULL, napi_default, NULL} +#define DECLARE_NODE_API_PROPERTY(name, func) \ + { (name), NULL, (func), NULL, NULL, NULL, napi_default, NULL } -#define DECLARE_NODE_API_GETTER(name, func) \ - {(name), NULL, NULL, (func), NULL, NULL, napi_default, NULL} +#define DECLARE_NODE_API_GETTER(name, func) \ + { (name), NULL, NULL, (func), NULL, NULL, napi_default, NULL } -#define DECLARE_NODE_API_PROPERTY_VALUE(name, value) \ - {(name), NULL, NULL, NULL, NULL, (value), napi_default, NULL} +#define DECLARE_NODE_API_PROPERTY_VALUE(name, value) \ + { (name), NULL, NULL, NULL, NULL, (value), napi_default, NULL } -static inline void add_returned_status(napi_env env, const char *key, +static inline void add_returned_status(napi_env env, + const char* key, napi_value object, - char *expected_message, + char* expected_message, napi_status expected_status, napi_status actual_status); -static inline void add_last_status(napi_env env, const char *key, +static inline void add_last_status(napi_env env, + const char* key, napi_value return_value); #include "common-inl.h" typedef node_api_nogc_env node_api_basic_env; -#endif // JS_NATIVE_API_COMMON_H_ +#endif // JS_NATIVE_API_COMMON_H_ From 2afb5e635d48ff0c400c91f01ea7f0d60e685dc5 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Thu, 14 Nov 2024 14:24:52 -0800 Subject: [PATCH 3/9] Skip compiling Node NAPI tests that we skip running --- test/napi/node-napi.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/napi/node-napi.test.ts b/test/napi/node-napi.test.ts index a38cdcb3c7..8664882d35 100644 --- a/test/napi/node-napi.test.ts +++ b/test/napi/node-napi.test.ts @@ -52,8 +52,9 @@ const failingNodeApiTests = [ beforeAll(async () => { const directories = jsNativeApiTests + .filter(t => !failingJsNativeApiTests.includes(t)) .map(t => join(jsNativeApiRoot, t)) - .concat(nodeApiTests.map(t => join(nodeApiRoot, t))) + .concat(nodeApiTests.filter(t => !failingNodeApiTests.includes(t)).map(t => join(nodeApiRoot, t))) .map(t => dirname(t)); const uniqueDirectories = Array.from(new Set(directories)); From 90852a37d5116e54bd4a765f373eac3a61554005 Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Thu, 14 Nov 2024 14:41:20 -0800 Subject: [PATCH 4/9] Use correct path separators on Windows --- test/napi/node-napi.test.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/test/napi/node-napi.test.ts b/test/napi/node-napi.test.ts index 8664882d35..6fbd5691ae 100644 --- a/test/napi/node-napi.test.ts +++ b/test/napi/node-napi.test.ts @@ -4,8 +4,8 @@ import { bunEnv, bunExe } from "harness"; import { join, dirname } from "path"; import os from "node:os"; -const jsNativeApiRoot = join(__dirname, "node-napi-tests/test/js-native-api"); -const nodeApiRoot = join(__dirname, "node-napi-tests/test/node-api"); +const jsNativeApiRoot = join(__dirname, "node-napi-tests", "test", "js-native-api"); +const nodeApiRoot = join(__dirname, "node-napi-tests", "test", "node-api"); const jsNativeApiTests = Array.from(new Glob("**/*.js").scanSync(jsNativeApiRoot)); const nodeApiTests = Array.from(new Glob("**/*.js").scanSync(nodeApiRoot)); @@ -50,6 +50,15 @@ const failingNodeApiTests = [ "test_worker_terminate/test.js", ]; +if (process.platform == "win32") { + for (const i in failingJsNativeApiTests) { + failingJsNativeApiTests[i] = failingJsNativeApiTests[i].replaceAll("/", "\\"); + } + for (const i in failingNodeApiTests) { + failingNodeApiTests[i] = failingNodeApiTests[i].replaceAll("/", "\\"); + } +} + beforeAll(async () => { const directories = jsNativeApiTests .filter(t => !failingJsNativeApiTests.includes(t)) From f50114332ff285e52954f5bd037b989ad1fb88e8 Mon Sep 17 00:00:00 2001 From: Kai Tamkun Date: Thu, 14 Nov 2024 15:09:26 -0800 Subject: [PATCH 5/9] Fix incorrect calling convention usage --- src/bun.js/javascript.zig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bun.js/javascript.zig b/src/bun.js/javascript.zig index 69130c9603..cd48b67c8a 100644 --- a/src/bun.js/javascript.zig +++ b/src/bun.js/javascript.zig @@ -428,9 +428,9 @@ pub export fn Bun__GlobalObject__hasIPC(global: *JSC.JSGlobalObject) bool { } extern fn Bun__Process__queueNextTick1(*JSC.ZigGlobalObject, JSC.JSValue, JSC.JSValue) void; -extern fn Bun__queueFinishNapiFinalizers(?*JSC.JSGlobalObject) callconv(.C) bool; +extern fn Bun__queueFinishNapiFinalizers(?*JSC.JSGlobalObject) bool; -pub export fn Bun__isNapiFinalizerQueueEmpty(globalObject: *JSGlobalObject) callconv(JSC.conv) bool { +pub export fn Bun__isNapiFinalizerQueueEmpty(globalObject: *JSGlobalObject) bool { return globalObject.bunVM().eventLoop().napi_finalizer_queue.count == 0; } From f9718af6a5121ca631de60edcc3832d693ac85f7 Mon Sep 17 00:00:00 2001 From: Kai Tamkun Date: Thu, 14 Nov 2024 15:17:28 -0800 Subject: [PATCH 6/9] Make checkGC fail if running a finalizer during napi env cleanup --- src/bun.js/bindings/napi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bun.js/bindings/napi.h b/src/bun.js/bindings/napi.h index 20e7f51a70..9638d7ae9a 100644 --- a/src/bun.js/bindings/napi.h +++ b/src/bun.js/bindings/napi.h @@ -160,7 +160,7 @@ public: void checkGC() { - if (UNLIKELY(m_globalObject->vm().heap.mutatorState() == JSC::MutatorState::Sweeping)) { + if (UNLIKELY(m_globalObject->vm().heap.mutatorState() == JSC::MutatorState::Sweeping || m_inCleanup)) { RELEASE_ASSERT_NOT_REACHED_WITH_MESSAGE( "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" From f73ef54edd9cc2e533a58662849d63cb097f11fb Mon Sep 17 00:00:00 2001 From: Ben Grant Date: Thu, 14 Nov 2024 15:58:55 -0800 Subject: [PATCH 7/9] Compile Node tests using Node 23 headers --- test/napi/node-napi.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/napi/node-napi.test.ts b/test/napi/node-napi.test.ts index 6fbd5691ae..02662b208e 100644 --- a/test/napi/node-napi.test.ts +++ b/test/napi/node-napi.test.ts @@ -74,7 +74,7 @@ beforeAll(async () => { stderr: "pipe", stdout: "ignore", stdin: "inherit", - env: bunEnv, + env: { ...bunEnv, npm_config_target: "v23.2.0" }, }); await process.exited; if (process.exitCode !== 0) { From 4103b738ff39ca6df25237cbaeb7f7a205c01718 Mon Sep 17 00:00:00 2001 From: Kai Tamkun Date: Thu, 14 Nov 2024 17:56:03 -0800 Subject: [PATCH 8/9] Report NAPI assertion failures more forcefully --- src/bun.js/bindings/napi.h | 40 ++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/src/bun.js/bindings/napi.h b/src/bun.js/bindings/napi.h index 9638d7ae9a..817b5d0eaf 100644 --- a/src/bun.js/bindings/napi.h +++ b/src/bun.js/bindings/napi.h @@ -40,6 +40,22 @@ struct napi_async_cleanup_hook_handle__ { } }; +#define NAPI_PERISH(...) \ + do { \ + WTFReportError(__FILE__, __LINE__, __PRETTY_FUNCTION__, __VA_ARGS__); \ + WTFReportBacktrace(); \ + WTFCrash(); \ + } while (0) + +#define NAPI_RELEASE_ASSERT(assertion, ...) \ + do { \ + if (UNLIKELY(!(assertion))) { \ + WTFReportAssertionFailureWithMessage(__FILE__, __LINE__, __PRETTY_FUNCTION__, #assertion, __VA_ARGS__); \ + WTFReportBacktrace(); \ + WTFCrash(); \ + } \ + } while (0) + // Named this way so we can manipulate napi_env values directly (since napi_env is defined as a pointer to struct napi_env__) struct napi_env__ { public: @@ -112,9 +128,7 @@ public: void addCleanupHook(void (*function)(void*), void* data) { for (const auto& [existing_function, existing_data] : m_cleanupHooks) { - if (UNLIKELY(function == existing_function && data == existing_data)) { - RELEASE_ASSERT_NOT_REACHED_WITH_MESSAGE("Attempted to add a duplicate NAPI environment cleanup hook"); - } + NAPI_RELEASE_ASSERT(function != existing_function || data != existing_data, "Attempted to add a duplicate NAPI environment cleanup hook"); } m_cleanupHooks.emplace_back(function, data); @@ -129,15 +143,13 @@ public: } } - RELEASE_ASSERT_NOT_REACHED_WITH_MESSAGE("Attempted to remove a NAPI environment cleanup hook that had never been added"); + NAPI_PERISH("Attempted to remove a NAPI environment cleanup hook that had never been added"); } napi_async_cleanup_hook_handle addAsyncCleanupHook(napi_async_cleanup_hook function, void* data) { for (const auto& [existing_function, existing_data, existing_handle] : m_asyncCleanupHooks) { - if (UNLIKELY(function == existing_function && data == existing_data)) { - RELEASE_ASSERT_NOT_REACHED_WITH_MESSAGE("Attempted to add a duplicate async NAPI environment cleanup hook"); - } + NAPI_RELEASE_ASSERT(function != existing_function || data != existing_data, "Attempted to add a duplicate async NAPI environment cleanup hook"); } auto iter = m_asyncCleanupHooks.emplace(m_asyncCleanupHooks.end(), function, data); @@ -155,18 +167,16 @@ public: } } - RELEASE_ASSERT_NOT_REACHED_WITH_MESSAGE("Attempted to remove an async NAPI environment cleanup hook that had never been added"); + NAPI_PERISH("Attempted to remove an async NAPI environment cleanup hook that had never been added"); } void checkGC() { - if (UNLIKELY(m_globalObject->vm().heap.mutatorState() == JSC::MutatorState::Sweeping || m_inCleanup)) { - RELEASE_ASSERT_NOT_REACHED_WITH_MESSAGE( - "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); - } + NAPI_RELEASE_ASSERT(m_globalObject->vm().heap.mutatorState() != JSC::MutatorState::Sweeping, + "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); } void doFinalizer(napi_finalize finalize_cb, void* data, void* finalize_hint) From 2aee62382fc0507810f70dcf436ca18cf9683183 Mon Sep 17 00:00:00 2001 From: Kai Tamkun Date: Thu, 14 Nov 2024 17:57:52 -0800 Subject: [PATCH 9/9] Remove async cleanup hooks from the list after they're called, not before --- src/bun.js/bindings/napi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bun.js/bindings/napi.h b/src/bun.js/bindings/napi.h index 817b5d0eaf..86e2993b90 100644 --- a/src/bun.js/bindings/napi.h +++ b/src/bun.js/bindings/napi.h @@ -84,10 +84,10 @@ public: while (!m_asyncCleanupHooks.empty()) { auto [function, data, handle] = m_asyncCleanupHooks.back(); - m_asyncCleanupHooks.pop_back(); ASSERT(function != nullptr); function(handle, data); delete handle; + m_asyncCleanupHooks.pop_back(); } for (const BoundFinalizer& boundFinalizer : m_finalizers) {