Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions docs/runtime/ffi.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ const {
},
});

const searchIterator = new JSCallback((ptr, length) => /hello/.test(new CString(ptr, length)), {
const searchIterator = new JSCallback((ptr, length) => /hello/.test(new CString(ptr, 0, length)), {
returns: "bool",
args: ["ptr", "usize"],
});
Expand All @@ -315,12 +315,18 @@ When you're done with a JSCallback, you should call `close()` to free the memory

Currently, thread-safe callbacks work best when run from another thread that is running JavaScript code, i.e. a [`Worker`](/runtime/workers). A future version of Bun will enable them to be called from any thread (such as new threads spawned by your native library that Bun is not aware of).

A thread-safe callback is dispatched to the JavaScript thread asynchronously, so it has no way to hand a result back to the native caller. `returns` must be `"void"` (the default); any other return type throws at construction.

```ts
const searchIterator = new JSCallback((ptr, length) => /hello/.test(new CString(ptr, length)), {
returns: "bool",
args: ["ptr", "usize"],
threadsafe: true, // Optional. Defaults to `false`
});
const onMatch = new JSCallback(
(ptr, length) => {
console.log(new CString(ptr, 0, length));
},
Comment thread
robobun marked this conversation as resolved.
{
args: ["ptr", "usize"],
threadsafe: true, // Optional. Defaults to `false`
},
);
```

<Note>
Expand Down
9 changes: 9 additions & 0 deletions packages/bun-types/ffi.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,10 @@ declare module "bun:ffi" {
* performance penalty needs to be less than the performance gain from
* running the function in a separate thread.
*
* A thread-safe callback is dispatched to the JavaScript thread
* asynchronously, so it cannot return a value to the native caller.
* `returns` must be `"void"` (the default).
*
* @default false
*/
readonly threadsafe?: boolean;
Expand Down Expand Up @@ -1106,6 +1110,11 @@ declare module "bun:ffi" {
* If called multiple times, does nothing after the first call.
*/
close(): void;

/**
* Calls {@link JSCallback.prototype.close}, so `using` releases the callback.
*/
[Symbol.dispose](): void;
}

/**
Expand Down
7 changes: 4 additions & 3 deletions src/js/bun/ffi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,10 @@ delete ffi.closeCallback;

class JSCallback {
constructor(cb, options) {
const { ctx, ptr } = nativeCallback(options, cb);
this.#ctx = ctx;
this.ptr = ptr;
const result = nativeCallback(options, cb);
if (Error.isError(result)) throw result;
this.#ctx = result.ctx;
this.ptr = result.ptr;
this.#threadsafe = !!options?.threadsafe;
}

Expand Down
4 changes: 3 additions & 1 deletion src/jsc/bindings/helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,9 @@ static const WTF::String toStringStatic(ZigString str)

static JSC::JSValue getErrorInstance(const ZigString* str, JSC::JSGlobalObject* globalObject)
{
WTF::String message = toString(*str);
// Must copy (like the sibling get*ErrorInstance helpers): `toString` uses
// StringImpl::createWithoutCopying, but the Error outlives the caller's bytes.
WTF::String message = toStringCopy(*str);
if (message.isNull() && str->len > 0) [[unlikely]] {
// pending exception while creating an error.
return {};
Expand Down
27 changes: 19 additions & 8 deletions src/runtime/ffi/ffi_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1816,12 +1816,6 @@ pub(super) fn generate_symbol_for_function(
));
}

if function.threadsafe && return_type != ABIType::Void {
return Ok(Some(
ZigString::static_(b"Threadsafe functions must return void").to_error_instance(global),
));
}

*function = Function::default();
function.base_name = None;
function.arg_types = abi_types;
Expand Down Expand Up @@ -2091,10 +2085,25 @@ impl Function {
is_threadsafe: bool,
) -> Result<(), bun_core::Error> {
jsc::mark_binding();
// FFI_Callback_threadsafe_call is void: it posts a task and returns
// before the JS function runs, so there is no return-value channel. A
// non-void trampoline would read an uninitialized EncodedJSValue.
if is_threadsafe && self.return_type != ABIType::Void {
self.fail(b"Threadsafe functions must return void");
return Ok(());
}
let mut source_code: Vec<u8> = Vec::new();
// SAFETY: js_context/js_function are live for the call
let ffi_wrapper = unsafe { Bun__createFFICallbackFunction(js_context, js_function) };
Comment thread
robobun marked this conversation as resolved.
self.print_callback_source_code(Some(js_context), Some(ffi_wrapper), &mut source_code)?;
// Heap-allocated wrapper with two JSC::Strong roots. Ownership
// transfers to Step::Compiled on success; on every earlier exit
// (TCCMissing, Step::Failed) this guard derefs it instead of leaking.
let ffi_wrapper = scopeguard::guard(ffi_wrapper, |p| {
// SAFETY: p came from Bun__createFFICallbackFunction and was not
// stored in Step::Compiled, so we still hold the +1.
unsafe { FFICallbackFunctionWrapper_destroy(p) };
});
self.print_callback_source_code(Some(js_context), Some(*ffi_wrapper), &mut source_code)?;

#[cfg(all(debug_assertions, unix))]
'debug_write: {
Expand Down Expand Up @@ -2221,7 +2230,9 @@ impl Function {
// dereferenced or written through on the Rust side; stored as
// NonNull to avoid laundering &T → *mut T provenance.
js_context: Some(NonNull::from(js_context)),
ffi_callback_function_wrapper: NonNull::new(ffi_wrapper),
ffi_callback_function_wrapper: NonNull::new(scopeguard::ScopeGuard::into_inner(
ffi_wrapper,
)),
});
Ok(())
}
Expand Down
6 changes: 0 additions & 6 deletions src/runtime/ffi/host_fns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,6 @@ pub fn generate_symbol_for_function(
))));
}

if function.threadsafe && return_type != ABIType::Void {
return Ok(Some(global.create_error_instance(format_args!(
"Threadsafe functions must return void"
))));
}

// `Function` has a `Drop` impl, so functional-record-update
// (`..Default::default()`) is rejected (E0509). Reset to default and assign
// the parsed fields individually instead.
Expand Down
43 changes: 42 additions & 1 deletion test/js/bun/ffi/ffi-error-messages.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { dlopen, linkSymbols } from "bun:ffi";
import { dlopen, JSCallback, linkSymbols } from "bun:ffi";
import { describe, expect, test } from "bun:test";
import { isArm64, isMusl, isWindows } from "harness";

Expand Down Expand Up @@ -86,4 +86,45 @@ describe.skipIf(isFFIUnavailable)("FFI error messages", () => {
});
}).toThrow('you must provide a "ptr" field with the memory address of the native function.');
});

describe("JSCallback", () => {
// A threadsafe callback is dispatched to the JS thread asynchronously, so
// it has no return-value channel: the trampoline would otherwise hand the
// native caller an uninitialized EncodedJSValue.
test.each(["u64", "int", "i64", "bool", "f64", "ptr", "cstring"] as const)(
"threadsafe: true with returns: %s throws at construction",
returns => {
expect(() => {
new JSCallback(() => 0, { args: ["i64"], returns, threadsafe: true });
}).toThrow("Threadsafe functions must return void");
},
);

test("threadsafe: true with an omitted return type still constructs", () => {
using cb = new JSCallback(() => {}, { args: ["i64"], threadsafe: true });
expect(cb.ptr).toBeGreaterThan(0);
});

test("threadsafe: true with returns: 'void' still constructs", () => {
using cb = new JSCallback(() => {}, { args: ["i64"], returns: "void", threadsafe: true });
expect(cb.ptr).toBeGreaterThan(0);
});

test("non-threadsafe callbacks may still return non-void", () => {
using cb = new JSCallback(x => x, { args: ["u64"], returns: "u64" });
expect(cb.ptr).toBeGreaterThan(0);
});

// JSCallback must throw native validation errors, like dlopen/cc/linkSymbols.
test.each([
["returns: buffer", { returns: "buffer" }, "Cannot return a buffer to JavaScript"],
["returns: napi_env", { returns: "napi_env" }, "Cannot return napi_env to JavaScript"],
["unknown arg type", { args: ["not_a_real_type"] }, "Unknown type not_a_real_type"],
["unknown return type", { returns: "not_a_real_type" }, "Unknown return type not_a_real_type"],
])("%s throws at construction", (_name, options, message) => {
expect(() => {
new JSCallback(() => {}, options as any);
}).toThrow(message);
});
});
});
Loading