diff --git a/src/jsc/bindings/JSFFIFunction.cpp b/src/jsc/bindings/JSFFIFunction.cpp index d5c2fd50dc3c..1be44d0a04ce 100644 --- a/src/jsc/bindings/JSFFIFunction.cpp +++ b/src/jsc/bindings/JSFFIFunction.cpp @@ -26,6 +26,7 @@ #include "root.h" #include "JSFFIFunction.h" +#include #include #include #include "ZigGlobalObject.h" @@ -205,23 +206,70 @@ FFI_Callback_call(FFICallbackFunctionWrapper& wrapper, size_t argCount, JSC::Enc return invokeFFICallback(wrapper.globalObject.get(), wrapper.m_function.get(), arguments); } +// Must match ABIType in src/runtime/ffi/abi_type.rs. +enum class FFIABIType : uint8_t { + int64_t_ = 7, + uint64_t_ = 8, + i64_fast = 15, + u64_fast = 16, +}; + +// For threadsafe callbacks, 64-bit integer arguments are passed through as +// raw bits because the TCC-generated trampoline may run on an arbitrary OS +// thread and must not heap-allocate a JSBigInt there. Convert them here, +// which always runs on the JS thread. +static inline JSC::JSValue decodeThreadsafeCallbackArgument(JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue raw, uint8_t abiType) +{ + switch (static_cast(abiType)) { + case FFIABIType::int64_t_: + return JSC::JSBigInt::createFrom(globalObject, static_cast(raw)); + case FFIABIType::uint64_t_: + return JSC::JSBigInt::createFrom(globalObject, static_cast(raw)); + case FFIABIType::i64_fast: { + int64_t val = static_cast(raw); + if (val >= JSC::minSafeInteger() && val <= JSC::maxSafeInteger()) + return JSC::jsNumber(val); + return JSC::JSBigInt::createFrom(globalObject, val); + } + case FFIABIType::u64_fast: { + uint64_t val = static_cast(raw); + if (val <= static_cast(JSC::maxSafeInteger())) + return JSC::jsNumber(static_cast(val)); + return JSC::JSBigInt::createFrom(globalObject, val); + } + default: + return JSC::JSValue::decode(raw); + } +} + extern "C" void -FFI_Callback_threadsafe_call(FFICallbackFunctionWrapper& wrapper, size_t argCount, JSC::EncodedJSValue* args) +FFI_Callback_threadsafe_call(FFICallbackFunctionWrapper& wrapper, size_t argCount, JSC::EncodedJSValue* args, const uint8_t* argTypes) { // Runs on a foreign thread: do not touch the wrapper's JSC::Strong members here. WTF::Vector argsVec; - for (size_t i = 0; i < argCount; ++i) + WTF::Vector argTypesVec; + for (size_t i = 0; i < argCount; ++i) { argsVec.append(args[i]); + argTypesVec.append(argTypes[i]); + } // Ref only once the context is found live (inside the map lock) and release via // adoptRef in the task, so the last deref — destroying two JSC::Strong members — // can only happen on the JS thread. On a dead/terminating context nothing is destroyed here. - WebCore::ScriptExecutionContext::postTaskTo(wrapper.m_contextId, [&wrapper] { wrapper.ref(); }, [argsVec = WTF::move(argsVec), wrapper = &wrapper](WebCore::ScriptExecutionContext& ctx) mutable { + WebCore::ScriptExecutionContext::postTaskTo(wrapper.m_contextId, [&wrapper] { wrapper.ref(); }, [argsVec = WTF::move(argsVec), argTypesVec = WTF::move(argTypesVec), wrapper = &wrapper](WebCore::ScriptExecutionContext& ctx) mutable { auto protectedWrapper = adoptRef(*wrapper); auto* globalObject = uncheckedDowncast(ctx.jsGlobalObject()); + auto& vm = JSC::getVM(globalObject); + auto scope = DECLARE_THROW_SCOPE(vm); JSC::MarkedArgumentBuffer arguments; - for (size_t i = 0; i < argsVec.size(); ++i) - arguments.appendWithCrashOnOverflow(JSC::JSValue::decode(argsVec[i])); + for (size_t i = 0; i < argsVec.size(); ++i) { + // JSBigInt::createFrom can throw (e.g. OOM). Leave the exception + // pending on the VM, matching invokeFFICallback. + JSC::JSValue arg = decodeThreadsafeCallbackArgument(globalObject, argsVec[i], argTypesVec[i]); + RETURN_IF_EXCEPTION(scope, void()); + arguments.appendWithCrashOnOverflow(arg); + } + scope.release(); invokeFFICallback(globalObject, protectedWrapper->m_function.get(), arguments); }); } diff --git a/src/runtime/ffi/FFI.h b/src/runtime/ffi/FFI.h index 52f803d9acb6..097b0c18f513 100644 --- a/src/runtime/ffi/FFI.h +++ b/src/runtime/ffi/FFI.h @@ -146,6 +146,15 @@ typedef void* JSContext; #ifdef IS_CALLBACK void* callback_ctx; +#ifdef IS_THREADSAFE +// Threadsafe callbacks may be invoked from an arbitrary OS thread. Converting +// 64-bit integer arguments to JSValue may need to heap-allocate a JSBigInt, +// which must only happen on the JS thread. The trampoline therefore passes +// the raw 64-bit bits for such arguments along with a per-argument ABIType +// tag so FFI_Callback_threadsafe_call can perform the conversion inside the +// task it posts to the JS thread. +BUN_FFI_IMPORT void FFI_Callback_call(void* ctx, size_t argCount, ZIG_REPR_TYPE* args, const uint8_t* argTypes); +#else BUN_FFI_IMPORT ZIG_REPR_TYPE FFI_Callback_call(void* ctx, size_t argCount, ZIG_REPR_TYPE* args); // We wrap static EncodedJSValue _FFI_Callback_call(void* ctx, size_t argCount, ZIG_REPR_TYPE* args) __attribute__((__always_inline__)); @@ -155,6 +164,7 @@ static EncodedJSValue _FFI_Callback_call(void* ctx, size_t argCount, ZIG_REPR_TY return return_value; } #endif +#endif static bool JSVALUE_IS_CELL(EncodedJSValue val) __attribute__((__always_inline__)); static bool JSVALUE_IS_INT32(EncodedJSValue val) __attribute__((__always_inline__)); @@ -353,7 +363,7 @@ static EncodedJSValue UINT64_TO_JSVALUE(void* jsGlobalObject, uint64_t val) { return INT32_TO_JSVALUE((int32_t)val); } - if (val < MAX_INT52) { + if (val <= MAX_INT52) { return DOUBLE_TO_JSVALUE((double)val); } @@ -361,7 +371,7 @@ static EncodedJSValue UINT64_TO_JSVALUE(void* jsGlobalObject, uint64_t val) { } static EncodedJSValue INT64_TO_JSVALUE(void* jsGlobalObject, int64_t val) { - if (val >= -MAX_INT32 && val <= MAX_INT32) { + if (val >= -MAX_INT32 && val < MAX_INT32) { return INT32_TO_JSVALUE((int32_t)val); } diff --git a/src/runtime/ffi/abi_type.rs b/src/runtime/ffi/abi_type.rs index 3c17b5c05274..0291bd664d2f 100644 --- a/src/runtime/ffi/abi_type.rs +++ b/src/runtime/ffi/abi_type.rs @@ -241,6 +241,17 @@ impl ABIType { matches!(self, ABIType::Double | ABIType::Float) } + /// Returns true if converting this type to a JSValue may heap-allocate + /// a JS object (a JSBigInt). For threadsafe callbacks the trampoline + /// may run on an arbitrary OS thread, so the allocation must be + /// deferred to the JS thread inside `FFI_Callback_threadsafe_call`. + pub fn may_allocate_bigint_when_converted_to_js(self) -> bool { + matches!( + self, + ABIType::Int64T | ABIType::Uint64T | ABIType::I64Fast | ABIType::U64Fast + ) + } + pub fn to_c(self, symbol: &[u8]) -> ToCFormatter<'_> { ToCFormatter { tag: self, diff --git a/src/runtime/ffi/ffi_body.rs b/src/runtime/ffi/ffi_body.rs index 0027fad813d5..8f54e8bc17da 100644 --- a/src/runtime/ffi/ffi_body.rs +++ b/src/runtime/ffi/ffi_body.rs @@ -1814,12 +1814,25 @@ pub(super) fn generate_symbol_for_function( )); } - if function.threadsafe && return_type != ABIType::Void { + if threadsafe && return_type != ABIType::Void { return Ok(Some( ZigString::static_(b"Threadsafe functions must return void").to_error_instance(global), )); } + if threadsafe { + for arg in abi_types.iter() { + if matches!(arg, ABIType::NapiEnv | ABIType::NapiValue) { + return Ok(Some( + ZigString::static_( + b"Threadsafe callbacks cannot accept napi_env or napi_value arguments", + ) + .to_error_instance(global), + )); + } + } + } + *function = Function::default(); function.base_name = None; function.arg_types = abi_types; @@ -2388,6 +2401,9 @@ impl Function { } writer.write_all(b"#define IS_CALLBACK 1\n")?; + if self.threadsafe { + writer.write_all(b"#define IS_THREADSAFE 1\n")?; + } 'brk: { if self.return_type.is_floating_point() { @@ -2444,12 +2460,43 @@ impl Function { for (i, arg) in self.arg_types.iter().enumerate() { let printed = bun_core::fmt::print_int(&mut arg_buf[3..], i); let arg_name = &arg_buf[0..3 + printed]; - writeln!( + if self.threadsafe && arg.may_allocate_bigint_when_converted_to_js() { + // The trampoline for a threadsafe callback may run on an + // arbitrary OS thread. Converting a 64-bit integer here + // would call {U,}INT64_TO_JSVALUE_SLOW, which allocates a + // JSBigInt on the calling thread without the JS lock and + // corrupts the GC heap. Pass the raw bits through instead + // and let FFI_Callback_threadsafe_call convert them on the + // JS thread using the argTypes table emitted below. + writeln!( + writer, + "arguments[{}] = (ZIG_REPR_TYPE)(int64_t){};", + i, + BStr::new(arg_name) + )?; + } else { + writeln!( + writer, + "arguments[{}] = {}.asZigRepr;", + i, + arg.to_js(arg_name) + )?; + } + } + + if self.threadsafe { + write!( writer, - "arguments[{}] = {}.asZigRepr;", - i, - arg.to_js(arg_name) + "static const uint8_t argTypes[{}] = {{", + self.arg_types.len() )?; + for (i, arg) in self.arg_types.iter().enumerate() { + if i > 0 { + writer.write_all(b", ")?; + } + write!(writer, "{}", *arg as i32)?; + } + writer.write_all(b"};\n")?; } } @@ -2461,7 +2508,24 @@ impl Function { let ptr = context_ptr.map(|p| p as usize).unwrap_or(0); let fmt = bun_fmt::hex_int_upper::<16>(ptr as u64); - let written = if !self.arg_types.is_empty() { + let written = if self.threadsafe { + let mut cursor = std::io::Cursor::new(&mut inner_buf_[1..]); + if !self.arg_types.is_empty() { + write!( + &mut cursor, + "FFI_Callback_call((void*)0x{}ULL, {}, arguments, argTypes)", + fmt, + self.arg_types.len() + )?; + } else { + write!( + &mut cursor, + "FFI_Callback_call((void*)0x{}ULL, 0, (ZIG_REPR_TYPE*)0, (const uint8_t*)0)", + fmt + )?; + } + cursor.position() as usize + } else if !self.arg_types.is_empty() { let mut cursor = std::io::Cursor::new(&mut inner_buf_[1..]); write!( &mut cursor, @@ -2513,7 +2577,7 @@ unsafe extern "C" { fn FFI_Callback_call_3(_: *mut c_void, _: usize, _: *mut JSValue) -> JSValue; fn FFI_Callback_call_4(_: *mut c_void, _: usize, _: *mut JSValue) -> JSValue; fn FFI_Callback_call_5(_: *mut c_void, _: usize, _: *mut JSValue) -> JSValue; - fn FFI_Callback_threadsafe_call(_: *mut c_void, _: usize, _: *mut JSValue) -> JSValue; + fn FFI_Callback_threadsafe_call(_: *mut c_void, _: usize, _: *mut JSValue, _: *const u8); fn FFI_Callback_call_6(_: *mut c_void, _: usize, _: *mut JSValue) -> JSValue; fn FFI_Callback_call_7(_: *mut c_void, _: usize, _: *mut JSValue) -> JSValue; fn Bun__createFFICallbackFunction(_: &JSGlobalObject, _: JSValue) -> *mut c_void; diff --git a/src/runtime/ffi/host_fns.rs b/src/runtime/ffi/host_fns.rs index 42b0f9cdfbf9..00f430edd919 100644 --- a/src/runtime/ffi/host_fns.rs +++ b/src/runtime/ffi/host_fns.rs @@ -123,12 +123,22 @@ pub fn generate_symbol_for_function( )))); } - if function.threadsafe && return_type != ABIType::Void { + if threadsafe && return_type != ABIType::Void { return Ok(Some(global.create_error_instance(format_args!( "Threadsafe functions must return void" )))); } + if threadsafe { + for arg in abi_types.iter() { + if matches!(arg, ABIType::NapiEnv | ABIType::NapiValue) { + return Ok(Some(global.create_error_instance(format_args!( + "Threadsafe callbacks cannot accept napi_env or napi_value arguments" + )))); + } + } + } + // `Function` has a `Drop` impl, so functional-record-update // (`..Default::default()`) is rejected (E0509). Reset to default and assign // the parsed fields individually instead. @@ -369,6 +379,9 @@ impl Function { } writer.write_all(b"#define IS_CALLBACK 1\n")?; + if self.threadsafe { + writer.write_all(b"#define IS_THREADSAFE 1\n")?; + } 'brk: { if self.return_type.is_floating_point() { @@ -423,12 +436,43 @@ impl Function { for (i, arg) in self.arg_types.iter().enumerate() { let printed = bun_core::fmt::print_int(&mut arg_buf[3..], i); let arg_name = &arg_buf[0..3 + printed]; - writeln!( + if self.threadsafe && arg.may_allocate_bigint_when_converted_to_js() { + // The trampoline for a threadsafe callback may run on an + // arbitrary OS thread. Converting a 64-bit integer here + // would call {U,}INT64_TO_JSVALUE_SLOW, which allocates a + // JSBigInt on the calling thread without the JS lock and + // corrupts the GC heap. Pass the raw bits through instead + // and let FFI_Callback_threadsafe_call convert them on the + // JS thread using the argTypes table emitted below. + writeln!( + writer, + "arguments[{}] = (ZIG_REPR_TYPE)(int64_t){};", + i, + BStr::new(arg_name) + )?; + } else { + writeln!( + writer, + "arguments[{}] = {}.asZigRepr;", + i, + arg.to_js(arg_name) + )?; + } + } + + if self.threadsafe { + write!( writer, - "arguments[{}] = {}.asZigRepr;", - i, - arg.to_js(arg_name) + "static const uint8_t argTypes[{}] = {{", + self.arg_types.len() )?; + for (i, arg) in self.arg_types.iter().enumerate() { + if i > 0 { + writer.write_all(b", ")?; + } + write!(writer, "{}", *arg as i32)?; + } + writer.write_all(b"};\n")?; } } @@ -438,7 +482,22 @@ impl Function { let written = { let ptr = context_ptr.map(|p| p as usize).unwrap_or(0); let mut cursor = std::io::Cursor::new(&mut inner_buf_[1..]); - if !self.arg_types.is_empty() { + if self.threadsafe { + if !self.arg_types.is_empty() { + write!( + &mut cursor, + "FFI_Callback_call((void*)0x{:X}ULL, {}, arguments, argTypes)", + ptr, + self.arg_types.len() + )?; + } else { + write!( + &mut cursor, + "FFI_Callback_call((void*)0x{:X}ULL, 0, (ZIG_REPR_TYPE*)0, (const uint8_t*)0)", + ptr + )?; + } + } else if !self.arg_types.is_empty() { write!( &mut cursor, "FFI_Callback_call((void*)0x{:X}ULL, {}, arguments)", diff --git a/test/js/bun/ffi/ffi-threadsafe-callback-bigint.test.ts b/test/js/bun/ffi/ffi-threadsafe-callback-bigint.test.ts new file mode 100644 index 000000000000..4373188dc751 --- /dev/null +++ b/test/js/bun/ffi/ffi-threadsafe-callback-bigint.test.ts @@ -0,0 +1,153 @@ +import { JSCallback } from "bun:ffi"; +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, isArm64, isMacOS, isWindows, tempDir } from "harness"; +import path from "node:path"; + +// TinyCC (and all of bun:ffi) is disabled on Windows ARM64. +const isFFIUnavailable = isWindows && isArm64; + +// There is no system `cc` on Windows x64 in CI. The bug being covered — +// heap-allocating a JSBigInt off the JS thread without the JS lock — is +// platform-independent, so exercising it on POSIX is sufficient. +const canRun = !isWindows; + +// The guard for this combination used to read the wrong variable and never +// fired, so the threadsafe codegen path (which cannot return a value from a +// posted task) was reachable. +test.skipIf(isFFIUnavailable)("threadsafe JSCallback with a non-void return type is rejected", () => { + expect(() => new JSCallback(() => 42, { returns: "int32_t", threadsafe: true })).toThrow( + "Threadsafe functions must return void", + ); +}); + +// A napi_env/napi_value is only valid on the JS thread that created it, so +// a threadsafe callback (invoked from an arbitrary OS thread) receiving one +// is inherently unsafe. +test.skipIf(isFFIUnavailable)("threadsafe JSCallback with napi_env / napi_value args is rejected", () => { + for (const t of ["napi_env", "napi_value"]) { + expect(() => new JSCallback(() => {}, { args: [t], threadsafe: true })).toThrow( + "Threadsafe callbacks cannot accept napi_env or napi_value arguments", + ); + } +}); + +// A threadsafe JSCallback with int64_t / uint64_t arguments used to convert +// those arguments to JSBigInt inside the TCC-generated trampoline, *before* +// posting to the JS thread. When the callback was invoked from a real OS +// thread that meant calling JSBigInt::createFrom without holding the JS +// lock, corrupting the GC heap. The trampoline now marshals the raw 64-bit +// bits and FFI_Callback_threadsafe_call converts them on the JS thread. +test.skipIf(!canRun)( + "threadsafe JSCallback with int64_t/uint64_t args invoked from a native thread", + async () => { + const srcDir = import.meta.dir; + const libName = isMacOS ? "libtscbbigint.dylib" : "libtscbbigint.so"; + + using dir = tempDir("ffi-threadsafe-cb-bigint", {}); + const outDir = String(dir); + const libPath = path.join(outDir, libName); + + { + const cmd = ["cc", "-shared", "-fPIC", "-o", libPath]; + if (!isMacOS) cmd.push("-pthread"); + cmd.push(path.join(srcDir, "threadsafe-callback-bigint.c")); + await using cc = Bun.spawn({ cmd, stderr: "pipe", stdout: "ignore" }); + const [stderr, exitCode] = await Promise.all([cc.stderr.text(), cc.exited]); + if (exitCode !== 0) { + throw new Error("failed to compile threadsafe-callback-bigint.c: " + stderr); + } + } + + const fixture = /* js */ ` + const { dlopen, JSCallback } = require("bun:ffi"); + + const lib = dlopen(${JSON.stringify(libPath)}, { + call_i64_from_thread: { args: ["ptr", "int64_t", "int32_t"], returns: "void" }, + call_u64_from_thread: { args: ["ptr", "uint64_t", "int32_t"], returns: "void" }, + call_mixed_from_thread: { args: ["ptr", "int32_t"], returns: "void" }, + }); + + const count = 50; + const errors = []; + + const cases = [ + // [label, abiType, caller, value, expected] + // Values outside the safe-integer range so the old trampoline would + // have to allocate a JSBigInt on the calling (non-JS) thread. + ["int64_t", "int64_t", "call_i64_from_thread", -9007199254740993n, -9007199254740993n], + ["uint64_t", "uint64_t", "call_u64_from_thread", 18446744073709551615n, 18446744073709551615n], + // i64_fast / u64_fast should return a number when the value fits in a + // double, and a BigInt otherwise. + ["i64_fast big", "i64_fast", "call_i64_from_thread", -9007199254740993n, -9007199254740993n], + ["i64_fast small", "i64_fast", "call_i64_from_thread", 123n, 123], + ["u64_fast big", "u64_fast", "call_u64_from_thread", 18446744073709551615n, 18446744073709551615n], + ["u64_fast small", "u64_fast", "call_u64_from_thread", 123n, 123], + ]; + + for (const [label, abiType, caller, value, expected] of cases) { + let received = 0; + const cb = new JSCallback( + (v) => { + if (v !== expected) { + errors.push(label + ": got " + String(v) + " (" + typeof v + "), expected " + String(expected) + " (" + typeof expected + ")"); + } + received++; + }, + { args: [abiType], returns: "void", threadsafe: true }, + ); + + lib.symbols[caller](cb.ptr, value, count); + + while (received < count) { + await new Promise((resolve) => setImmediate(resolve)); + } + cb.close(); + } + + // Mixed args: verify non-64-bit args are still decoded as normal JSValues + // alongside deferred 64-bit conversions. + { + let received = 0; + const cb = new JSCallback( + (a, b, c, d) => { + if (a !== 42) errors.push("mixed a: " + String(a)); + if (b !== -9007199254740993n) errors.push("mixed b: " + String(b)); + if (c !== 18446744073709551615n) errors.push("mixed c: " + String(c)); + if (d !== 3.5) errors.push("mixed d: " + String(d)); + received++; + }, + { args: ["int32_t", "int64_t", "uint64_t", "double"], returns: "void", threadsafe: true }, + ); + lib.symbols.call_mixed_from_thread(cb.ptr, count); + while (received < count) { + await new Promise((resolve) => setImmediate(resolve)); + } + cb.close(); + } + + // Force a full GC so a corrupted MarkedBlock free list (from off-thread + // JSBigInt allocation) is detected before the process exits cleanly. + Bun.gc(true); + + lib.close(); + + if (errors.length > 0) { + console.error(errors.slice(0, 10).join("\\n")); + process.exit(1); + } + + console.log("OK"); + `; + + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", fixture], + env: bunEnv, + stderr: "pipe", + stdout: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ stdout: "OK", stderr: "", exitCode: 0 }); + }, + 30_000, +); diff --git a/test/js/bun/ffi/ffi.test.fixture.callback.c b/test/js/bun/ffi/ffi.test.fixture.callback.c index f6d6d40cae96..3819981cd47e 100644 --- a/test/js/bun/ffi/ffi.test.fixture.callback.c +++ b/test/js/bun/ffi/ffi.test.fixture.callback.c @@ -148,6 +148,15 @@ typedef void* JSContext; #ifdef IS_CALLBACK void* callback_ctx; +#ifdef IS_THREADSAFE +// Threadsafe callbacks may be invoked from an arbitrary OS thread. Converting +// 64-bit integer arguments to JSValue may need to heap-allocate a JSBigInt, +// which must only happen on the JS thread. The trampoline therefore passes +// the raw 64-bit bits for such arguments along with a per-argument ABIType +// tag so FFI_Callback_threadsafe_call can perform the conversion inside the +// task it posts to the JS thread. +BUN_FFI_IMPORT void FFI_Callback_call(void* ctx, size_t argCount, ZIG_REPR_TYPE* args, const uint8_t* argTypes); +#else BUN_FFI_IMPORT ZIG_REPR_TYPE FFI_Callback_call(void* ctx, size_t argCount, ZIG_REPR_TYPE* args); // We wrap static EncodedJSValue _FFI_Callback_call(void* ctx, size_t argCount, ZIG_REPR_TYPE* args) __attribute__((__always_inline__)); @@ -157,6 +166,7 @@ static EncodedJSValue _FFI_Callback_call(void* ctx, size_t argCount, ZIG_REPR_TY return return_value; } #endif +#endif static bool JSVALUE_IS_CELL(EncodedJSValue val) __attribute__((__always_inline__)); static bool JSVALUE_IS_INT32(EncodedJSValue val) __attribute__((__always_inline__)); @@ -355,7 +365,7 @@ static EncodedJSValue UINT64_TO_JSVALUE(void* jsGlobalObject, uint64_t val) { return INT32_TO_JSVALUE((int32_t)val); } - if (val < MAX_INT52) { + if (val <= MAX_INT52) { return DOUBLE_TO_JSVALUE((double)val); } @@ -363,7 +373,7 @@ static EncodedJSValue UINT64_TO_JSVALUE(void* jsGlobalObject, uint64_t val) { } static EncodedJSValue INT64_TO_JSVALUE(void* jsGlobalObject, int64_t val) { - if (val >= -MAX_INT32 && val <= MAX_INT32) { + if (val >= -MAX_INT32 && val < MAX_INT32) { return INT32_TO_JSVALUE((int32_t)val); } diff --git a/test/js/bun/ffi/ffi.test.fixture.receiver.c b/test/js/bun/ffi/ffi.test.fixture.receiver.c index af6a78283716..1e4f43fd0d12 100644 --- a/test/js/bun/ffi/ffi.test.fixture.receiver.c +++ b/test/js/bun/ffi/ffi.test.fixture.receiver.c @@ -148,6 +148,15 @@ typedef void* JSContext; #ifdef IS_CALLBACK void* callback_ctx; +#ifdef IS_THREADSAFE +// Threadsafe callbacks may be invoked from an arbitrary OS thread. Converting +// 64-bit integer arguments to JSValue may need to heap-allocate a JSBigInt, +// which must only happen on the JS thread. The trampoline therefore passes +// the raw 64-bit bits for such arguments along with a per-argument ABIType +// tag so FFI_Callback_threadsafe_call can perform the conversion inside the +// task it posts to the JS thread. +BUN_FFI_IMPORT void FFI_Callback_call(void* ctx, size_t argCount, ZIG_REPR_TYPE* args, const uint8_t* argTypes); +#else BUN_FFI_IMPORT ZIG_REPR_TYPE FFI_Callback_call(void* ctx, size_t argCount, ZIG_REPR_TYPE* args); // We wrap static EncodedJSValue _FFI_Callback_call(void* ctx, size_t argCount, ZIG_REPR_TYPE* args) __attribute__((__always_inline__)); @@ -157,6 +166,7 @@ static EncodedJSValue _FFI_Callback_call(void* ctx, size_t argCount, ZIG_REPR_TY return return_value; } #endif +#endif static bool JSVALUE_IS_CELL(EncodedJSValue val) __attribute__((__always_inline__)); static bool JSVALUE_IS_INT32(EncodedJSValue val) __attribute__((__always_inline__)); @@ -355,7 +365,7 @@ static EncodedJSValue UINT64_TO_JSVALUE(void* jsGlobalObject, uint64_t val) { return INT32_TO_JSVALUE((int32_t)val); } - if (val < MAX_INT52) { + if (val <= MAX_INT52) { return DOUBLE_TO_JSVALUE((double)val); } @@ -363,7 +373,7 @@ static EncodedJSValue UINT64_TO_JSVALUE(void* jsGlobalObject, uint64_t val) { } static EncodedJSValue INT64_TO_JSVALUE(void* jsGlobalObject, int64_t val) { - if (val >= -MAX_INT32 && val <= MAX_INT32) { + if (val >= -MAX_INT32 && val < MAX_INT32) { return INT32_TO_JSVALUE((int32_t)val); } diff --git a/test/js/bun/ffi/threadsafe-callback-bigint.c b/test/js/bun/ffi/threadsafe-callback-bigint.c new file mode 100644 index 000000000000..5fca2cfaacb2 --- /dev/null +++ b/test/js/bun/ffi/threadsafe-callback-bigint.c @@ -0,0 +1,84 @@ +#include +#include + +#ifdef _WIN32 +#define FFI_EXPORT __declspec(dllexport) +#define WIN32_LEAN_AND_MEAN +#include +#else +#define FFI_EXPORT __attribute__((visibility("default"))) +#include +#endif + +typedef void (*cb_i64_t)(int64_t); +typedef void (*cb_u64_t)(uint64_t); +typedef void (*cb_mixed_t)(int32_t, int64_t, uint64_t, double); + +struct i64_args { cb_i64_t cb; int64_t value; int32_t count; }; +struct u64_args { cb_u64_t cb; uint64_t value; int32_t count; }; +struct mixed_args { cb_mixed_t cb; int32_t count; }; + +#ifdef _WIN32 +#define THREAD_T HANDLE +#define THREAD_RETURN DWORD WINAPI +#define THREAD_RETVAL 0 +#define THREAD_CREATE(t, fn, arg) ((t) = CreateThread(NULL, 0, fn, arg, 0, NULL)) +#define THREAD_JOIN(t) do { WaitForSingleObject((t), INFINITE); CloseHandle((t)); } while (0) +#else +#define THREAD_T pthread_t +#define THREAD_RETURN void * +#define THREAD_RETVAL NULL +#define THREAD_CREATE(t, fn, arg) pthread_create(&(t), NULL, fn, arg) +#define THREAD_JOIN(t) pthread_join((t), NULL) +#endif + +static THREAD_RETURN worker_i64(void *p) +{ + struct i64_args *a = (struct i64_args *)p; + for (int32_t i = 0; i < a->count; i++) a->cb(a->value); + return THREAD_RETVAL; +} + +static THREAD_RETURN worker_u64(void *p) +{ + struct u64_args *a = (struct u64_args *)p; + for (int32_t i = 0; i < a->count; i++) a->cb(a->value); + return THREAD_RETVAL; +} + +static THREAD_RETURN worker_mixed(void *p) +{ + struct mixed_args *a = (struct mixed_args *)p; + for (int32_t i = 0; i < a->count; i++) { + // values chosen so that the 64-bit args require JSBigInt + a->cb(42, (int64_t)-9007199254740993LL, (uint64_t)18446744073709551615ULL, 3.5); + } + return THREAD_RETVAL; +} + +// Invoke a threadsafe JSCallback taking an int64_t from a real OS thread. +// Blocks until the thread has finished, so by the time this returns all +// callback invocations have at least been posted to the JS thread's queue. +FFI_EXPORT void call_i64_from_thread(cb_i64_t cb, int64_t value, int32_t count) +{ + struct i64_args args = { cb, value, count }; + THREAD_T t; + THREAD_CREATE(t, worker_i64, &args); + THREAD_JOIN(t); +} + +FFI_EXPORT void call_u64_from_thread(cb_u64_t cb, uint64_t value, int32_t count) +{ + struct u64_args args = { cb, value, count }; + THREAD_T t; + THREAD_CREATE(t, worker_u64, &args); + THREAD_JOIN(t); +} + +FFI_EXPORT void call_mixed_from_thread(cb_mixed_t cb, int32_t count) +{ + struct mixed_args args = { cb, count }; + THREAD_T t; + THREAD_CREATE(t, worker_mixed, &args); + THREAD_JOIN(t); +}