Skip to content
Open
Show file tree
Hide file tree
Changes from 6 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
1 change: 0 additions & 1 deletion AGENTS.md

This file was deleted.

1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
CLAUDE.md
4 changes: 3 additions & 1 deletion scripts/build/rust.ts
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,9 @@ export function emitRust(n: Ninja, cfg: Config, inputs: RustBuildInputs): string
// `-fuse-ld=`. RUSTFLAGS only reach *target* crates when `--target` is given,
// and the `bun_bin` staticlib has no link step, so it's normally dead — but
// if a target cdylib ever appears it'd fail with "could not open '-fuse-ld=lld'".
if (!cfg.windows) rustflags.push(`-Clink-arg=-fuse-ld=lld`);
if (!cfg.windows && !cfg.darwin) {
rustflags.push(`-Clink-arg=-fuse-ld=lld`);
}
if (cfg.crossLangLto) {
// Cross-language LTO: emit LLVM bitcode (not machine code) into the .a
// so the final lld `-flto=full` link sees through Rust↔C++ call edges.
Expand Down
2 changes: 1 addition & 1 deletion src/jsc/bindings/InternalModuleRegistry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ ALWAYS_INLINE JSC::JSValue generateNativeModule(
#ifdef BUN_DYNAMIC_JS_LOAD_PATH
JSValue initializeInternalModuleFromDisk(JSGlobalObject* globalObject, VM& vm, const WTF::String& moduleName, WTF::String fileBase, const WTF::String& urlString)
{
WTF::String file = makeString(ASCIILiteral::fromLiteralUnsafe(BUN_DYNAMIC_JS_LOAD_PATH), "/"_s, WTF::move(fileBase));
WTF::String file = makeString(WTF::String::fromUTF8(BUN_DYNAMIC_JS_LOAD_PATH), "/"_s, WTF::move(fileBase));
if (auto contents = WTF::FileSystemImpl::readEntireFile(file)) {
auto string = WTF::String::fromUTF8(contents.value());
return generateModule(globalObject, vm, string, moduleName, urlString);
Expand Down
35 changes: 25 additions & 10 deletions src/jsc/bindings/JSFFIFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,25 +36,40 @@
#include "DOMJITIDLType.h"
#include "DOMJITIDLTypeFilter.h"
#include "DOMJITHelpers.h"
#include "ScriptExecutionContext.h"

class FFICallbackFunctionWrapper {
class FFICallbackFunctionWrapper final : public WTF::ThreadSafeRefCounted<FFICallbackFunctionWrapper> {

WTF_DEPRECATED_MAKE_FAST_ALLOCATED(FFICallbackFunctionWrapper);

public:
static Ref<FFICallbackFunctionWrapper> create(JSC::JSFunction* function, Zig::GlobalObject* globalObject)
{
return adoptRef(*new FFICallbackFunctionWrapper(function, globalObject));
}

JSC::Strong<JSC::JSFunction> m_function;
JSC::Strong<Zig::GlobalObject> globalObject;
~FFICallbackFunctionWrapper() = default;

WebCore::ScriptExecutionContextIdentifier contextIdentifier() const { return m_contextIdentifier; }

FFICallbackFunctionWrapper(JSC::JSFunction* function, Zig::GlobalObject* globalObject)
: m_function(globalObject->vm(), function)
, globalObject(globalObject->vm(), globalObject)
, m_contextIdentifier(globalObject->scriptExecutionContext()->identifier())
{
}
Comment on lines 56 to 61

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if scriptExecutionContext() can return nullptr in GlobalObject implementations
rg -nC3 'scriptExecutionContext\(\)' --type cpp | head -80

Repository: oven-sh/bun

Length of output: 5768


🏁 Script executed:

# Find Zig::GlobalObject implementation and scriptExecutionContext() definition
fd -t f 'GlobalObject\|Zig' --type h --type cpp | head -30

Repository: oven-sh/bun

Length of output: 224


🏁 Script executed:

# Search for Zig::GlobalObject class definition
rg -n 'class\s+GlobalObject' --type cpp --type h | head -20

Repository: oven-sh/bun

Length of output: 1263


🏁 Script executed:

# Look for scriptExecutionContext() implementation in GlobalObject
rg -nB2 -A5 'GlobalObject.*scriptExecutionContext\(\)' --type cpp --type h

Repository: oven-sh/bun

Length of output: 17952


🏁 Script executed:

# Check what calls FFICallbackFunctionWrapper constructor
rg -n 'FFICallbackFunctionWrapper' --type cpp | head -20

Repository: oven-sh/bun

Length of output: 2756


Add null check for scriptExecutionContext() on line 59.

The constructor dereferences globalObject->scriptExecutionContext()->identifier() without null checking. The method returns a raw pointer that can be nullptr, as evidenced by the widespread null-checking pattern throughout the codebase (JSAbortSignal.cpp, JSCallbackData.cpp, JSPerformanceObserver.cpp, and others all defensively check this pointer). Add a null check or assert to prevent a potential crash during FFICallbackFunctionWrapper construction.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/jsc/bindings/JSFFIFunction.cpp` around lines 56 - 61, The constructor
FFICallbackFunctionWrapper currently dereferences
globalObject->scriptExecutionContext() when initializing m_contextIdentifier;
add a defensive null check (or assert) for
globalObject->scriptExecutionContext() before using it so we don't crash when it
is nullptr—e.g., in the FFICallbackFunctionWrapper(JSC::JSFunction* function,
Zig::GlobalObject* globalObject) initializer or body, verify
scriptExecutionContext() is non-null and only call identifier() when valid (or
set m_contextIdentifier to a safe default/invalid value and log/assert) to match
the null-checking pattern used elsewhere (see uses in JSAbortSignal.cpp,
JSCallbackData.cpp, JSPerformanceObserver.cpp).


private:
friend class WTF::ThreadSafeRefCounted<FFICallbackFunctionWrapper>;

~FFICallbackFunctionWrapper() = default;

const WebCore::ScriptExecutionContextIdentifier m_contextIdentifier;
};
extern "C" void FFICallbackFunctionWrapper_destroy(FFICallbackFunctionWrapper* wrapper)
{
delete wrapper;
wrapper->deref();
}

extern "C" FFICallbackFunctionWrapper* Bun__createFFICallbackFunction(
Expand All @@ -66,9 +81,7 @@ extern "C" FFICallbackFunctionWrapper* Bun__createFFICallbackFunction(

auto* callbackFunction = uncheckedDowncast<JSC::JSFunction>(JSC::JSValue::decode(callbackFn));

auto* wrapper = new FFICallbackFunctionWrapper(callbackFunction, globalObject);

return wrapper;
return &FFICallbackFunctionWrapper::create(callbackFunction, globalObject).leakRef();
}

extern "C" Zig::JSFFIFunction* Bun__CreateFFIFunctionWithData(Zig::GlobalObject* globalObject, const ZigString* symbolName, unsigned argCount, Zig::FFIFunction functionPointer, void* data)
Expand Down Expand Up @@ -206,17 +219,19 @@ FFI_Callback_call(FFICallbackFunctionWrapper& wrapper, size_t argCount, JSC::Enc
extern "C" void
FFI_Callback_threadsafe_call(FFICallbackFunctionWrapper& wrapper, size_t argCount, JSC::EncodedJSValue* args)
{

auto* globalObject = wrapper.globalObject.get();
// This can run on an arbitrary native thread. Keep JSC::Strong access on the
// posted JS task; touching it here can race with GC's handle visitation.
Ref protectedWrapper { wrapper };
auto contextIdentifier = protectedWrapper->contextIdentifier();
WTF::Vector<JSC::EncodedJSValue, 8> argsVec;
for (size_t i = 0; i < argCount; ++i)
argsVec.append(args[i]);

WebCore::ScriptExecutionContext::postTaskTo(globalObject->scriptExecutionContext()->identifier(), [argsVec = WTF::move(argsVec), wrapper](WebCore::ScriptExecutionContext& ctx) mutable {
WebCore::ScriptExecutionContext::postTaskTo(contextIdentifier, [argsVec = WTF::move(argsVec), protectedWrapper = WTF::move(protectedWrapper)](WebCore::ScriptExecutionContext& ctx) mutable {
auto* globalObject = uncheckedDowncast<Zig::GlobalObject>(ctx.jsGlobalObject());
auto& vm = JSC::getVM(globalObject);
JSC::MarkedArgumentBuffer arguments;
auto* function = wrapper.m_function.get();
auto* function = protectedWrapper->m_function.get();
for (size_t i = 0; i < argsVec.size(); ++i)
arguments.appendWithCrashOnOverflow(JSC::JSValue::decode(argsVec[i]));
WTF::NakedPtr<JSC::Exception> exception;
Expand Down
112 changes: 111 additions & 1 deletion test/js/bun/ffi/cc.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { cc, CString, ptr, type FFIFunction, type Library } from "bun:ffi";
import { cc, CString, JSCallback, ptr, type FFIFunction, type Library } from "bun:ffi";
import { afterAll, beforeAll, describe, expect, it } from "bun:test";
import { promises as fs } from "fs";
import { bunEnv, bunExe, isArm64, isASAN, isWindows, tempDirWithFiles } from "harness";
Expand Down Expand Up @@ -96,6 +96,116 @@ describe.skipIf(isASAN || isFFIUnavailable)("given an add(a, b) function", () =>
});
}); // </given add(a, b) function>

describe.skipIf(isWindows || isASAN || isFFIUnavailable)("threadsafe JSCallback", () => {
const source = /* c */ `
typedef void (*callback_t)(int);

#ifdef __APPLE__
typedef struct _opaque_pthread_t* pthread_t;
#else
typedef unsigned long pthread_t;
#endif

extern int pthread_create(pthread_t*, const void*, void* (*)(void*), void*);
extern int pthread_detach(pthread_t);

static callback_t active_callback;
static int active_count;
static _Atomic int callbacks_finished;

static void* run_callbacks(void* unused) {
(void)unused;
for (int i = 0; i < active_count; i++) {
active_callback(i);
}
callbacks_finished = 1;
return (void*)0;
}

int start_threadsafe_callbacks(callback_t callback, int count) {
pthread_t thread;
active_callback = callback;
active_count = count;
callbacks_finished = 0;
if (pthread_create(&thread, (void*)0, run_callbacks, (void*)0) != 0) {
return -1;
}
pthread_detach(thread);
return count;
}

int threadsafe_callbacks_finished(void) {
return callbacks_finished;
}
`;

let dir: string;
let library: Library<{
start_threadsafe_callbacks: { args: ["ptr", "int"]; returns: "int" };
threadsafe_callbacks_finished: { args: []; returns: "int" };
}>;

beforeAll(() => {
dir = tempDirWithFiles("bun-ffi-threadsafe-callback-test", {
"callback.c": source,
});
library = cc({
source: path.join(dir, "callback.c"),
library: "pthread",
symbols: {
start_threadsafe_callbacks: {
returns: "int",
args: ["ptr", "int"],
},
threadsafe_callbacks_finished: {
returns: "int",
args: [],
},
},
});
});

afterAll(async () => {
library?.close();
if (dir) {
await fs.rm(dir, { recursive: true, force: true });
}
});

it("can be called repeatedly from a native thread while the JS thread runs GC", async () => {
const count = 4096;
const values: number[] = [];
const callback = new JSCallback(
value => {
values.push(value);
},
{
args: ["int"],
returns: "void",
threadsafe: true,
},
);

try {
expect(library.symbols.start_threadsafe_callbacks(callback.ptr, count)).toBe(count);

for (let i = 0; i < 4096 && values.length < count; i++) {
Bun.gc(true);
await Bun.sleep(0);
if (library.symbols.threadsafe_callbacks_finished() && values.length === count) {
break;
}
}

expect(library.symbols.threadsafe_callbacks_finished()).toBe(1);
expect(values).toHaveLength(count);
expect(values).toEqual(Array.from({ length: count }, (_, i) => i));
} finally {
callback.close();
}
});
});

describe("given a source file with syntax errors", () => {
const source = /* c */ `
int add(int a, int b) {
Expand Down