diff --git a/src/bun.js/bindings/AsyncContextFrame.cpp b/src/bun.js/bindings/AsyncContextFrame.cpp index 44de71206d..eafa120553 100644 --- a/src/bun.js/bindings/AsyncContextFrame.cpp +++ b/src/bun.js/bindings/AsyncContextFrame.cpp @@ -43,6 +43,11 @@ JSValue AsyncContextFrame::withAsyncContextIfNeeded(JSGlobalObject* globalObject return callback; } + // If already wrapped in an AsyncContextFrame, return as-is to avoid double-wrapping. + if (jsDynamicCast(callback)) { + return callback; + } + // Construct a low-overhead wrapper auto& vm = JSC::getVM(globalObject); return AsyncContextFrame::create( diff --git a/src/bun.js/bindings/napi.cpp b/src/bun.js/bindings/napi.cpp index e6a5dc54cb..60cf7df2d4 100644 --- a/src/bun.js/bindings/napi.cpp +++ b/src/bun.js/bindings/napi.cpp @@ -2427,6 +2427,11 @@ extern "C" napi_status napi_typeof(napi_env env, napi_value val, return napi_clear_last_error(env); } + if (JSC::jsDynamicCast(value)) { + *result = napi_function; + return napi_clear_last_error(env); + } + *result = napi_object; return napi_clear_last_error(env); diff --git a/src/napi/napi.zig b/src/napi/napi.zig index 5ab0bb54de..b2a53b6d6e 100644 --- a/src/napi/napi.zig +++ b/src/napi/napi.zig @@ -677,7 +677,7 @@ pub export fn napi_make_callback(env_: napi_env, _: *anyopaque, recv_: napi_valu return envIsNull(); }; const recv, const func = .{ recv_.get(), func_.get() }; - if (func.isEmptyOrUndefinedOrNull() or !func.isCallable()) { + if (func.isEmptyOrUndefinedOrNull() or (!func.isCallable() and !func.isAsyncContextFrame())) { return env.setLastError(.function_expected); } @@ -1762,7 +1762,7 @@ pub export fn napi_create_threadsafe_function( }; const func = func_.get(); - if (call_js_cb == null and (func.isEmptyOrUndefinedOrNull() or !func.isCallable())) { + if (call_js_cb == null and (func.isEmptyOrUndefinedOrNull() or (!func.isCallable() and !func.isAsyncContextFrame()))) { return env.setLastError(.function_expected); } diff --git a/test/napi/napi-app/module.js b/test/napi/napi-app/module.js index 7462614379..facabd3dd0 100644 --- a/test/napi/napi-app/module.js +++ b/test/napi/napi-app/module.js @@ -895,4 +895,52 @@ nativeTests.test_napi_typeof_boxed_primitives = () => { console.log("All boxed primitive tests passed!"); }; +// https://github.com/oven-sh/bun/issues/25933 +// Test that napi_typeof returns napi_function for callbacks wrapped in +// AsyncContextFrame (which happens inside AsyncLocalStorage.run()). +nativeTests.test_napi_typeof_async_context_frame = async () => { + const { AsyncLocalStorage } = require("node:async_hooks"); + const als = new AsyncLocalStorage(); + + await als.run({ key: "value" }, () => { + return new Promise(resolve => { + // Pass a callback to the native addon. Because we're inside + // AsyncLocalStorage.run(), Bun wraps it in AsyncContextFrame. + // The native call_js_cb will call napi_typeof on the received + // js_callback and print the result. + nativeTests.test_issue_25933(() => {}); + // The threadsafe function callback fires asynchronously. + setTimeout(resolve, 50); + }); + }); +}; + +// Test that napi_make_callback works when the func is an AsyncContextFrame +// (received by a threadsafe function's call_js_cb inside AsyncLocalStorage.run()). +nativeTests.test_make_callback_with_async_context = async () => { + const { AsyncLocalStorage } = require("node:async_hooks"); + const als = new AsyncLocalStorage(); + + await als.run({ key: "value" }, () => { + return new Promise(resolve => { + nativeTests.test_napi_make_callback_async_context_frame(() => {}); + setTimeout(resolve, 50); + }); + }); +}; + +// Test that napi_create_threadsafe_function with call_js_cb=NULL accepts an +// AsyncContextFrame as the func (received from another threadsafe function's call_js_cb). +nativeTests.test_create_tsfn_with_async_context = async () => { + const { AsyncLocalStorage } = require("node:async_hooks"); + const als = new AsyncLocalStorage(); + + await als.run({ key: "value" }, () => { + return new Promise(resolve => { + nativeTests.test_napi_create_tsfn_async_context_frame(() => {}); + setTimeout(resolve, 100); + }); + }); +}; + module.exports = nativeTests; diff --git a/test/napi/napi-app/standalone_tests.cpp b/test/napi/napi-app/standalone_tests.cpp index 46cefd4867..0fc3cf55de 100644 --- a/test/napi/napi-app/standalone_tests.cpp +++ b/test/napi/napi-app/standalone_tests.cpp @@ -1998,6 +1998,127 @@ static napi_value test_napi_get_named_property_copied_string(const Napi::Callbac return ok(env); } +// https://github.com/oven-sh/bun/issues/25933 +// When a threadsafe function is created inside AsyncLocalStorage.run(), +// the js_callback gets wrapped in AsyncContextFrame. napi_typeof must +// still report it as napi_function, not napi_object. +static napi_threadsafe_function tsfn_25933 = nullptr; + +static void test_issue_25933_callback(napi_env env, napi_value js_callback, + void *context, void *data) { + napi_valuetype type; + napi_status status = napi_typeof(env, js_callback, &type); + if (status != napi_ok) { + printf("FAIL: napi_typeof returned error status %d\n", status); + } else if (type == napi_function) { + printf("PASS: napi_typeof returned napi_function\n"); + } else { + printf("FAIL: napi_typeof returned %d, expected napi_function (%d)\n", + type, napi_function); + } + napi_release_threadsafe_function(tsfn_25933, napi_tsfn_release); + tsfn_25933 = nullptr; +} + +static napi_value test_issue_25933(const Napi::CallbackInfo &info) { + Napi::Env env = info.Env(); + Napi::HandleScope scope(env); + + // The first argument is the JS callback function. + // When called inside AsyncLocalStorage.run(), Bun wraps this in + // AsyncContextFrame via withAsyncContextIfNeeded. + napi_value js_cb = info[0]; + napi_value name = Napi::String::New(env, "tsfn_typeof_test"); + + NODE_API_CALL(env, + napi_create_threadsafe_function( + env, js_cb, nullptr, name, 0, 1, nullptr, nullptr, + nullptr, &test_issue_25933_callback, &tsfn_25933)); + NODE_API_CALL(env, napi_call_threadsafe_function(tsfn_25933, nullptr, + napi_tsfn_nonblocking)); + return env.Undefined(); +} + +// When a threadsafe function's call_js_cb receives a js_callback that is an +// AsyncContextFrame, calling napi_make_callback on it should work (not fail +// with function_expected). +static napi_threadsafe_function tsfn_make_callback = nullptr; + +static void test_make_callback_tsfn_cb(napi_env env, napi_value js_callback, + void *context, void *data) { + napi_value recv; + napi_get_global(env, &recv); + + napi_value result; + napi_status status = napi_make_callback(env, nullptr, recv, js_callback, 0, nullptr, &result); + if (status == napi_ok) { + printf("PASS: napi_make_callback succeeded\n"); + } else { + printf("FAIL: napi_make_callback returned status %d\n", status); + } + napi_release_threadsafe_function(tsfn_make_callback, napi_tsfn_release); + tsfn_make_callback = nullptr; +} + +static napi_value test_napi_make_callback_async_context_frame(const Napi::CallbackInfo &info) { + Napi::Env env = info.Env(); + Napi::HandleScope scope(env); + + napi_value js_cb = info[0]; + napi_value name = Napi::String::New(env, "tsfn_make_callback_test"); + + NODE_API_CALL(env, + napi_create_threadsafe_function( + env, js_cb, nullptr, name, 0, 1, nullptr, nullptr, + nullptr, &test_make_callback_tsfn_cb, &tsfn_make_callback)); + NODE_API_CALL(env, napi_call_threadsafe_function(tsfn_make_callback, nullptr, + napi_tsfn_nonblocking)); + return env.Undefined(); +} + +// When a threadsafe function's call_js_cb receives a js_callback that is an +// AsyncContextFrame, passing it to a second napi_create_threadsafe_function +// with call_js_cb=NULL should succeed (not fail with function_expected). +static napi_threadsafe_function tsfn_create_outer = nullptr; + +static void test_create_tsfn_outer_cb(napi_env env, napi_value js_callback, + void *context, void *data) { + // js_callback here is an AsyncContextFrame in Bun. + // Try to create a new threadsafe function with it and call_js_cb=NULL. + napi_value name; + napi_create_string_utf8(env, "inner_tsfn", NAPI_AUTO_LENGTH, &name); + + napi_threadsafe_function inner_tsfn = nullptr; + napi_status status = napi_create_threadsafe_function( + env, js_callback, nullptr, name, 0, 1, nullptr, nullptr, + nullptr, /* call_js_cb */ nullptr, &inner_tsfn); + if (status != napi_ok) { + printf("FAIL: napi_create_threadsafe_function returned status %d\n", status); + } else { + printf("PASS: napi_create_threadsafe_function accepted AsyncContextFrame\n"); + // Release immediately — we only needed to verify creation succeeds. + napi_release_threadsafe_function(inner_tsfn, napi_tsfn_release); + } + napi_release_threadsafe_function(tsfn_create_outer, napi_tsfn_release); + tsfn_create_outer = nullptr; +} + +static napi_value test_napi_create_tsfn_async_context_frame(const Napi::CallbackInfo &info) { + Napi::Env env = info.Env(); + Napi::HandleScope scope(env); + + napi_value js_cb = info[0]; + napi_value name = Napi::String::New(env, "tsfn_create_test"); + + NODE_API_CALL(env, + napi_create_threadsafe_function( + env, js_cb, nullptr, name, 0, 1, nullptr, nullptr, + nullptr, &test_create_tsfn_outer_cb, &tsfn_create_outer)); + NODE_API_CALL(env, napi_call_threadsafe_function(tsfn_create_outer, nullptr, + napi_tsfn_nonblocking)); + return env.Undefined(); +} + void register_standalone_tests(Napi::Env env, Napi::Object exports) { REGISTER_FUNCTION(env, exports, test_issue_7685); REGISTER_FUNCTION(env, exports, test_issue_11949); @@ -2033,6 +2154,9 @@ void register_standalone_tests(Napi::Env env, Napi::Object exports) { REGISTER_FUNCTION(env, exports, napi_get_typeof); REGISTER_FUNCTION(env, exports, test_external_buffer_data_lifetime); REGISTER_FUNCTION(env, exports, test_napi_get_named_property_copied_string); + REGISTER_FUNCTION(env, exports, test_issue_25933); + REGISTER_FUNCTION(env, exports, test_napi_make_callback_async_context_frame); + REGISTER_FUNCTION(env, exports, test_napi_create_tsfn_async_context_frame); } } // namespace napitests diff --git a/test/napi/napi.test.ts b/test/napi/napi.test.ts index 75d5ea81cc..e6390ad61f 100644 --- a/test/napi/napi.test.ts +++ b/test/napi/napi.test.ts @@ -798,6 +798,30 @@ describe("cleanup hooks", () => { expect(output).toContain("napi_typeof"); }); + it("should return napi_function for AsyncContextFrame in threadsafe callback", async () => { + // Test for https://github.com/oven-sh/bun/issues/25933 + // When a threadsafe function is created inside AsyncLocalStorage.run(), + // the callback gets wrapped in AsyncContextFrame. napi_typeof must + // report it as napi_function, not napi_object. + const output = await checkSameOutput("test_napi_typeof_async_context_frame", []); + expect(output).toContain("PASS: napi_typeof returned napi_function"); + }); + + it("should handle AsyncContextFrame in napi_make_callback", async () => { + // When a threadsafe function's call_js_cb receives an AsyncContextFrame + // as js_callback and passes it to napi_make_callback, it should succeed. + const output = await checkSameOutput("test_make_callback_with_async_context", []); + expect(output).toContain("PASS: napi_make_callback succeeded"); + }); + + it("should accept AsyncContextFrame in napi_create_threadsafe_function with null call_js_cb", async () => { + // When a threadsafe function's call_js_cb receives an AsyncContextFrame + // and passes it to a second napi_create_threadsafe_function with + // call_js_cb=NULL, it should not reject with function_expected. + const output = await checkSameOutput("test_create_tsfn_with_async_context", []); + expect(output).toContain("PASS: napi_create_threadsafe_function accepted AsyncContextFrame"); + }); + it("should return napi_object for boxed primitives (String, Number, Boolean)", async () => { // Regression test for https://github.com/oven-sh/bun/issues/25351 // napi_typeof was incorrectly returning napi_string for String objects (new String("hello"))