Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 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
13 changes: 7 additions & 6 deletions src/bun.js/bindings/libuv/generate_uv_posix_stubs_constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ export const symbols = [
"uv_handle_size",
"uv_handle_type_name",
"uv_has_ref",
// Defined in uv-posix-polyfills.cpp
// Defined in uv-posix-polyfills.c
// "uv_hrtime",
"uv_idle_init",
"uv_idle_start",
Expand Down Expand Up @@ -162,15 +162,15 @@ export const symbols = [
"uv_loop_size",
"uv_metrics_idle_time",
"uv_metrics_info",
// Defined in uv-posix-polyfills.cpp
// Defined in uv-posix-polyfills.c
// "uv_mutex_destroy",
// "uv_mutex_init",
// "uv_mutex_init_recursive",
// "uv_mutex_lock",
// "uv_mutex_trylock",
// "uv_mutex_unlock",
"uv_now",
// Defined in uv-posix-polyfills.cpp
// Defined in uv-posix-polyfills.c
// "uv_once",
"uv_open_osfhandle",
"uv_os_environ",
Expand All @@ -182,9 +182,9 @@ export const symbols = [
"uv_os_get_passwd2",
"uv_os_getenv",
"uv_os_gethostname",
// Defined in uv-posix-polyfills.cpp
// Defined in uv-posix-polyfills.c
// "uv_os_getpid",
// Defined in uv-posix-polyfills.cpp
// Defined in uv-posix-polyfills.c
// "uv_os_getppid",
"uv_os_getpriority",
"uv_os_homedir",
Expand Down Expand Up @@ -280,7 +280,8 @@ export const symbols = [
"uv_thread_getname",
"uv_thread_getpriority",
"uv_thread_join",
"uv_thread_self",
// Defined in uv-posix-polyfills.c
// "uv_thread_self",
"uv_thread_setaffinity",
"uv_thread_setname",
"uv_thread_setpriority",
Expand Down
7 changes: 7 additions & 0 deletions src/bun.js/bindings/uv-posix-polyfills.c
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ UV_EXTERN void uv_once(uv_once_t* guard, void (*callback)(void))
abort();
}

// Copy-pasted from libuv (src/unix/thread.c).
// uv_thread_t is pthread_t on POSIX (see uv/unix.h).
UV_EXTERN uv_thread_t uv_thread_self(void)
{
return pthread_self();
}

UV_EXTERN uint64_t uv_hrtime(void)
{
return uv__hrtime(UV_CLOCK_PRECISE);
Expand Down
6 changes: 0 additions & 6 deletions src/bun.js/bindings/uv-posix-stubs.c
Original file line number Diff line number Diff line change
Expand Up @@ -1745,12 +1745,6 @@ UV_EXTERN int uv_thread_join(uv_thread_t* tid)
__builtin_unreachable();
}

UV_EXTERN uv_thread_t uv_thread_self(void)
{
__bun_throw_not_implemented("uv_thread_self");
__builtin_unreachable();
}

UV_EXTERN int uv_thread_setaffinity(uv_thread_t* tid,
char* cpumask,
char* oldmask,
Expand Down
6 changes: 0 additions & 6 deletions test/napi/uv-stub-stuff/plugin.c
Original file line number Diff line number Diff line change
Expand Up @@ -2089,12 +2089,6 @@ napi_value call_uv_func(napi_env env, napi_callback_info info) {
return NULL;
}

if (strcmp(buffer, "uv_thread_self") == 0) {

uv_thread_self();
return NULL;
}

if (strcmp(buffer, "uv_thread_setaffinity") == 0) {
uv_thread_t *arg0 = {0};
char *arg1 = {0};
Expand Down
20 changes: 20 additions & 0 deletions test/napi/uv-stub-stuff/uv_impl.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include <node_api.h>

#include <pthread.h>
#include <signal.h>
#include <stdio.h>
#include <string.h>
Expand Down Expand Up @@ -102,6 +103,22 @@ static napi_value test_uv_once(napi_env env, napi_callback_info info) {
return ret;
}

// Test uv_thread_self — regression test for #29223.
// ffi-napi calls uv_thread_self during NAPI module init; Bun used to panic
// because uv_thread_self was a stubbed libuv symbol on POSIX.
static napi_value test_thread_self(napi_env env, napi_callback_info info) {
// Call uv_thread_self twice — both from the current (main) thread. They
// must refer to the same thread per pthread_equal.
uv_thread_t a = uv_thread_self();
uv_thread_t b = uv_thread_self();

napi_value ret;
// pthread_equal returns non-zero for equal threads. Return a boolean so
// the JS side can check it without caring about uv_thread_t layout.
napi_get_boolean(env, pthread_equal(a, b) != 0, &ret);
return ret;
}
Comment thread
robobun marked this conversation as resolved.

// Test uv_hrtime
static napi_value test_hrtime(napi_env env, napi_callback_info info) {
uint64_t time1 = uv_hrtime();
Expand Down Expand Up @@ -150,6 +167,9 @@ napi_value Init(napi_env env, napi_value exports) {
napi_create_function(env, NULL, 0, test_uv_once, NULL, &fn);
napi_set_named_property(env, exports, "testUvOnce", fn);

napi_create_function(env, NULL, 0, test_thread_self, NULL, &fn);
napi_set_named_property(env, exports, "testThreadSelf", fn);

napi_create_function(env, NULL, 0, test_hrtime, NULL, &fn);
napi_set_named_property(env, exports, "testHrtime", fn);

Expand Down
6 changes: 6 additions & 0 deletions test/napi/uv.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,12 @@ describe.if(!isWindows)("uv stubs", () => {
expect(nativeModule.testUvOnce()).toBe(1);
});

// Regression test for #29223: ffi-napi calls uv_thread_self during NAPI
// module init. Used to panic because it was a stubbed libuv symbol.
test("uv_thread_self (#29223)", () => {
expect(nativeModule.testThreadSelf()).toBe(true);
});

test("hrtime", () => {
const result = nativeModule.testHrtime();

Expand Down
105 changes: 105 additions & 0 deletions test/regression/issue/29223.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
// Regression test for https://github.com/oven-sh/bun/issues/29223
//
// Issue: `ffi-napi`'s NAPI addon calls `uv_thread_self()` from inside its
// module-init path. `uv_thread_self` was a stubbed libuv symbol on POSIX,
// so Bun would panic with "unsupported uv function: uv_thread_self" and
// the addon (and therefore the user's program) crashed before a single
// FFI call was made.
//
// Fix: implement `uv_thread_self` on POSIX as a pthread_self() wrapper
// (matches upstream libuv src/unix/thread.c).
//
// This regression test builds a minimal NAPI addon that calls
// `uv_thread_self()` from its Init function — the exact shape of the
// ffi-napi crash — and asserts that requiring it does not crash.

Check warning on line 14 in test/regression/issue/29223.test.ts

View check run for this annotation

Claude / Claude Code Review

Multi-line prose comment block in regression test

Lines 2–14 of test/regression/issue/29223.test.ts contain a 13-line prose block (// Issue: ..., // Fix: ..., and test-design narrative) that violates the project convention: regression test files should carry only the single-line GitHub issue URL comment. Lines 2–14 should be deleted; the URL on line 1 is correct and should be kept.
Comment thread
robobun marked this conversation as resolved.
Outdated
import { beforeAll, describe, expect, test } from "bun:test";
import { bunEnv, bunExe, isWindows, makeTree, tempDirWithFiles } from "harness";
import path from "node:path";

// Windows uses real libuv; the POSIX-stub code path does not apply there.
describe.if(!isWindows)("issue #29223", () => {
let tempdir: string = "";

// Build the addon once in beforeAll (same pattern as test/napi/uv.test.ts).
beforeAll(async () => {
const addonSource = `
#include <node_api.h>
#include <pthread.h>
#include <uv.h>

napi_value Init(napi_env env, napi_value exports) {
// This is what ffi-napi does: call uv_thread_self() while the NAPI
// module is being constructed. Before the fix this panicked Bun.
uv_thread_t self = uv_thread_self();

// Also check that calling it twice from the same thread agrees with
// pthread_self() — proves we actually implemented it rather than
// returning a garbage value.
uv_thread_t again = uv_thread_self();
int equal = pthread_equal(self, again) != 0 && pthread_equal(self, pthread_self()) != 0;

napi_value equal_js;
napi_get_boolean(env, equal, &equal_js);
napi_set_named_property(env, exports, "equal", equal_js);
return exports;
}

NAPI_MODULE(NODE_GYP_MODULE_NAME, Init)
`;

const files = {
"addon.c": addonSource,
"package.json": JSON.stringify({
name: "issue-29223-addon",
version: "0.0.0",
private: true,
scripts: { "build:napi": "node-gyp configure && node-gyp build" },
dependencies: { "node-gyp": "10.2.0" },
}),
"binding.gyp": `{
"targets": [
{
"target_name": "addon",
"sources": [ "addon.c" ],
"include_dirs": [ ".", "./libuv" ],
"cflags": ["-fPIC"],
"ldflags": ["-Wl,--export-dynamic"]
}
Comment thread
robobun marked this conversation as resolved.
]
}`,
"index.js": `const addon = require("./build/Release/addon.node");
if (addon.equal !== true) {
console.error("FAIL: uv_thread_self returned inconsistent results");
process.exit(2);
}
console.log("OK");
`,
};

tempdir = tempDirWithFiles("issue-29223", files);
await makeTree(tempdir, files);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

// node-gyp uses the libuv headers we vendor for the stubs.
const libuvDir = path.join(import.meta.dir, "../../../src/bun.js/bindings/libuv");
await Bun.$`cp -R ${libuvDir} ${path.join(tempdir, "libuv")}`.env(bunEnv);
await Bun.$`${bunExe()} install && ${bunExe()} run build:napi`.env(bunEnv).cwd(tempdir);
});

test("NAPI addon calling uv_thread_self during Init does not crash", () => {
// spawnSync because the baseline (pre-fix) crashes via panic + abort;
// spawn + proc.exited can hang on such aborts under the test runner.
const { stdout, exitCode } = Bun.spawnSync({
cmd: [bunExe(), "index.js"],
env: bunEnv,
cwd: tempdir,
stderr: "pipe",
stdout: "pipe",
});

// The addon prints "OK" from index.js only if require() succeeded and
// uv_thread_self() returned a thread id consistent with pthread_self().
// Pre-fix, require() panics the process and stdout stays empty.
expect(stdout.toString().trim()).toBe("OK");
expect(exitCode).toBe(0);
});
});
Loading