mirror of
https://github.com/oven-sh/bun
synced 2026-02-02 15:08:46 +00:00
fix(node): support duplicate dlopen calls with DLHandleMap (#24404)
## Summary Fixes an issue where loading the same native module (NODE_MODULE_CONTEXT_AWARE) multiple times would fail with: ``` symbol 'napi_register_module_v1' not found in native module ``` Fixes https://github.com/oven-sh/bun/issues/23136 Fixes https://github.com/oven-sh/bun/issues/21432 ## Root Cause When a native module is loaded for the first time: 1. `dlopen()` loads the shared library 2. Static constructors run and call `node_module_register()` 3. The module registers successfully On subsequent loads of the same module: 1. `dlopen()` returns the same handle (library already loaded) 2. Static constructors **do not run again** 3. No registration occurs, leading to the "symbol not found" error ## Solution Implemented a thread-safe `DLHandleMap` to cache and replay module registrations: 1. **Thread-local storage** captures the `node_module*` during static constructor execution 2. **After successful first load**, save the registration to the global map 3. **On subsequent loads**, look up the cached registration and replay it This approach matches Node.js's `global_handle_map` implementation. ## Changes - Created `src/bun.js/bindings/DLHandleMap.h` - thread-safe singleton cache - Added thread-local storage in `src/bun.js/bindings/v8/node.cpp` - Modified `src/bun.js/bindings/BunProcess.cpp` to save/lookup cached modules - Also includes the exports fix (using `toObject()` to match Node.js behavior) ## Test Plan Added `test/js/node/process/dlopen-duplicate-load.test.ts` with tests that: - Build a native addon using node-gyp - Load it twice with `process.dlopen` - Verify both loads succeed - Test with different exports objects All tests pass. ## Related Issue Fixes the second bug discovered in the segfault investigation. --------- Co-authored-by: Claude Bot <claude-bot@bun.sh> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
This commit is contained in:
148
test/js/node/process/dlopen-duplicate-load.test.ts
Normal file
148
test/js/node/process/dlopen-duplicate-load.test.ts
Normal file
@@ -0,0 +1,148 @@
|
||||
import { spawnSync } from "bun";
|
||||
import { beforeAll, describe, expect, test } from "bun:test";
|
||||
import { bunEnv, bunExe, tempDirWithFiles } from "harness";
|
||||
import { join } from "path";
|
||||
|
||||
// This test verifies that Bun can load the same native module multiple times
|
||||
// Previously, the second load would fail with "symbol 'napi_register_module_v1' not found"
|
||||
// because static constructors only run once, so the module registration wasn't replayed
|
||||
|
||||
describe("process.dlopen duplicate loads", () => {
|
||||
let addonPath: string;
|
||||
|
||||
beforeAll(() => {
|
||||
const addonSource = `
|
||||
#include <node.h>
|
||||
|
||||
namespace demo {
|
||||
|
||||
using v8::Context;
|
||||
using v8::FunctionCallbackInfo;
|
||||
using v8::Isolate;
|
||||
using v8::Local;
|
||||
using v8::Object;
|
||||
using v8::String;
|
||||
using v8::Value;
|
||||
|
||||
void Hello(const FunctionCallbackInfo<Value>& args) {
|
||||
Isolate* isolate = args.GetIsolate();
|
||||
args.GetReturnValue().Set(String::NewFromUtf8(isolate, "world").ToLocalChecked());
|
||||
}
|
||||
|
||||
void Initialize(Local<Object> exports,
|
||||
Local<Value> module,
|
||||
Local<Context> context,
|
||||
void* priv) {
|
||||
NODE_SET_METHOD(exports, "hello", Hello);
|
||||
}
|
||||
|
||||
} // namespace demo
|
||||
|
||||
NODE_MODULE_CONTEXT_AWARE(addon, demo::Initialize)
|
||||
`;
|
||||
|
||||
const bindingGyp = `
|
||||
{
|
||||
"targets": [
|
||||
{
|
||||
"target_name": "addon",
|
||||
"sources": [ "addon.cpp" ]
|
||||
}
|
||||
]
|
||||
}
|
||||
`;
|
||||
|
||||
const dir = tempDirWithFiles("dlopen-duplicate-test", {
|
||||
"addon.cpp": addonSource,
|
||||
"binding.gyp": bindingGyp,
|
||||
"package.json": JSON.stringify({
|
||||
name: "test",
|
||||
version: "1.0.0",
|
||||
gypfile: true,
|
||||
scripts: {
|
||||
install: "node-gyp rebuild",
|
||||
},
|
||||
devDependencies: {
|
||||
"node-gyp": "^11.2.0",
|
||||
},
|
||||
}),
|
||||
});
|
||||
|
||||
// Build the addon
|
||||
const build = spawnSync({
|
||||
cmd: [bunExe(), "install"],
|
||||
cwd: dir,
|
||||
env: bunEnv,
|
||||
stdout: "inherit",
|
||||
stderr: "inherit",
|
||||
});
|
||||
|
||||
if (!build.success) {
|
||||
throw new Error("Failed to build native addon");
|
||||
}
|
||||
|
||||
addonPath = join(dir, "build", "Release", "addon.node");
|
||||
});
|
||||
|
||||
test("should load the same module twice successfully", async () => {
|
||||
const testScript = `
|
||||
// First load
|
||||
const m1 = { exports: {} };
|
||||
process.dlopen(m1, "${addonPath.replace(/\\/g, "\\\\")}");
|
||||
console.log("First load: hello exists?", typeof m1.exports.hello === "function");
|
||||
|
||||
// Second load - this should work now
|
||||
const m2 = { exports: {} };
|
||||
process.dlopen(m2, "${addonPath.replace(/\\/g, "\\\\")}");
|
||||
console.log("Second load: hello exists?", typeof m2.exports.hello === "function");
|
||||
|
||||
// Verify both work
|
||||
console.log("First module result:", m1.exports.hello());
|
||||
console.log("Second module result:", m2.exports.hello());
|
||||
`;
|
||||
|
||||
await using proc = Bun.spawn({
|
||||
cmd: [bunExe(), "-e", testScript],
|
||||
env: bunEnv,
|
||||
stdout: "pipe",
|
||||
stderr: "pipe",
|
||||
});
|
||||
|
||||
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
|
||||
|
||||
expect(stdout).toContain("First load: hello exists? true");
|
||||
expect(stdout).toContain("Second load: hello exists? true");
|
||||
expect(stdout).toContain("First module result: world");
|
||||
expect(stdout).toContain("Second module result: world");
|
||||
expect(exitCode).toBe(0);
|
||||
});
|
||||
|
||||
test("should load module with different exports objects", async () => {
|
||||
const testScript = `
|
||||
// First load with empty object
|
||||
const m1 = { exports: {} };
|
||||
process.dlopen(m1, "${addonPath.replace(/\\/g, "\\\\")}");
|
||||
console.log("m1.exports.hello:", m1.exports.hello());
|
||||
|
||||
// Second load with different exports object
|
||||
const m2 = { exports: { initial: true } };
|
||||
process.dlopen(m2, "${addonPath.replace(/\\/g, "\\\\")}");
|
||||
console.log("m2.exports.initial:", m2.exports.initial);
|
||||
console.log("m2.exports.hello:", m2.exports.hello());
|
||||
`;
|
||||
|
||||
await using proc = Bun.spawn({
|
||||
cmd: [bunExe(), "-e", testScript],
|
||||
env: bunEnv,
|
||||
stdout: "pipe",
|
||||
stderr: "pipe",
|
||||
});
|
||||
|
||||
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
|
||||
|
||||
expect(stdout).toContain("m1.exports.hello: world");
|
||||
expect(stdout).toContain("m2.exports.initial: true");
|
||||
expect(stdout).toContain("m2.exports.hello: world");
|
||||
expect(exitCode).toBe(0);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user