fix(v8): export node::EmitAsyncInit and related async hooks C++ API symbols#28132
fix(v8): export node::EmitAsyncInit and related async hooks C++ API symbols#28132
Conversation
…ymbols Native addons compiled against Node.js headers (like libpq used by pg-native) reference node::EmitAsyncInit and related async hooks C++ API functions. Bun was missing these symbols, causing ERR_DLOPEN_FAILED when loading such addons. Add stub implementations for EmitAsyncInit, EmitAsyncDestroy, AsyncHooksGetExecutionAsyncId, and AsyncHooksGetTriggerAsyncId in the V8 compatibility layer. Closes #28131 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Updated 1:36 AM PT - Mar 15th, 2026
❌ Your commit
🧪 To try this PR locally: bunx bun-pr 28132That installs a local version of the PR into your bun-28132 --bun |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
WalkthroughIntroduces async hooks API support by adding five new functions to the node bindings: AsyncHooksGetExecutionAsyncId, AsyncHooksGetTriggerAsyncId, and overloaded EmitAsyncInit variants, along with EmitAsyncDestroy. Includes type definitions for async_id and async_context, exports these symbols across multiple symbol tables, and adds corresponding test coverage. Changes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
Comment |
|
Found 0 issues this PR may fix beyond #28131. After searching through the repository for open issues related to:
No additional open issues were found that would be directly fixed by this PR's implementation of async hooks C++ API stub functions (EmitAsyncInit, EmitAsyncDestroy, AsyncHooksGetExecutionAsyncId, AsyncHooksGetTriggerAsyncId). This PR specifically addresses issue #28131 as stated in the description. 🤖 Generated with Claude Code |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward stub implementations for missing V8 async hooks symbols, with proper symbol exports on all platforms and tests.
Extended reasoning...
Overview
This PR adds stub implementations for five node:: C++ API functions (AsyncHooksGetExecutionAsyncId, AsyncHooksGetTriggerAsyncId, two overloads of EmitAsyncInit, and EmitAsyncDestroy) to Bun's V8 compatibility layer. These are no-op stubs returning sentinel values (-1), matching the pattern of the existing AddEnvironmentCleanupHook/RemoveEnvironmentCleanupHook stubs. The mangled symbols are correctly added to all three platform-specific export files (symbols.txt for Linux, symbols.dyn for macOS, symbols.def for Windows). A test is added that exercises all new functions under both Bun and Node.js.
Security risks
None. The new functions are no-op stubs that return hardcoded values. They don't process user input, access the filesystem, or interact with the network. The async_context struct and async_id typedef are simple numeric types matching Node.js's definitions.
Level of scrutiny
This is a low-risk, mechanical change. The stub implementations follow the exact same pattern as existing stubs in the same file. The symbol export additions are straightforward name mangling entries. The test is a simple smoke test confirming the functions don't crash. This warrants minimal scrutiny.
Other factors
The one nit found (missing V8API entries in napi.zig) is a valid observation about internal consistency, but as the bug report itself acknowledges, the practical impact is negligible — the platform-specific export files are the primary mechanism ensuring symbol visibility, and the doNotOptimizeAway loop in napi.zig is a secondary belt-and-suspenders measure. The symbols will be exported correctly on all platforms without the napi.zig change. The PR includes a proper test that validates behavior matches Node.js, and it addresses a real user-facing issue (pg-native failing to load).
| async_id AsyncHooksGetExecutionAsyncId(v8::Isolate* isolate) | ||
| { | ||
| return -1; | ||
| } | ||
|
|
||
| async_id AsyncHooksGetTriggerAsyncId(v8::Isolate* isolate) | ||
| { | ||
| return -1; | ||
| } | ||
|
|
||
| async_context EmitAsyncInit(v8::Isolate* isolate, | ||
| v8::Local<v8::Object> resource, | ||
| const char* name, | ||
| async_id trigger_async_id) | ||
| { | ||
| return async_context { -1, -1 }; | ||
| } | ||
|
|
||
| async_context EmitAsyncInit(v8::Isolate* isolate, | ||
| v8::Local<v8::Object> resource, | ||
| v8::Local<v8::String> name, | ||
| async_id trigger_async_id) | ||
| { | ||
| return async_context { -1, -1 }; | ||
| } | ||
|
|
||
| void EmitAsyncDestroy(v8::Isolate* isolate, | ||
| async_context asyncContext) | ||
| { | ||
| } |
There was a problem hiding this comment.
🟡 Nit: The new async hooks functions (AsyncHooksGetExecutionAsyncId, AsyncHooksGetTriggerAsyncId, EmitAsyncInit x2, EmitAsyncDestroy) are added to the symbol export files but not to the V8API struct in src/napi/napi.zig. The existing node:: functions like AddEnvironmentCleanupHook (line 1839) and RemoveEnvironmentCleanupHook (line 1840) have entries in both Unix and Windows branches of this struct, and the project's own src/bun.js/bindings/v8/CLAUDE.md documents this as a required step. Adding the corresponding mangled symbol entries to the V8API struct would maintain consistency with the established pattern.
Extended reasoning...
What the bug is
The PR adds five new node:: C++ API functions (AsyncHooksGetExecutionAsyncId, AsyncHooksGetTriggerAsyncId, two overloads of EmitAsyncInit, and EmitAsyncDestroy) and correctly adds their mangled symbols to all three platform-specific export files (symbols.txt, symbols.dyn, symbols.def). However, it does not add corresponding entries to the V8API struct in src/napi/napi.zig.
The established pattern
The existing node::AddEnvironmentCleanupHook and node::RemoveEnvironmentCleanupHook functions follow a two-part pattern: their mangled symbols appear in (1) the platform export files AND (2) the V8API struct in napi.zig (lines 1839-1840 for Unix, lines 1926-1927 for Windows). The V8API struct is iterated at lines 2487-2488 with doNotOptimizeAway to prevent dead code elimination from stripping these symbols.
Documentation requirement
The project's own src/bun.js/bindings/v8/CLAUDE.md explicitly documents this as a required step (section 2a: "Add to src/napi/napi.zig") when implementing new V8/node C++ API functions.
Practical impact
In practice, the risk is low. The platform-specific symbol export files (symbols.txt for Linux, symbols.dyn for macOS, symbols.def for Windows) are the primary mechanism that ensures the linker retains these symbols. The doNotOptimizeAway loop in napi.zig is a secondary belt-and-suspenders mechanism. Since the symbols are correctly listed in all three export files and the functions are declared with BUN_EXPORT, they will almost certainly be exported correctly on all platforms.
Step-by-step proof
src/napi/napi.zigline 1835 definesconst V8API = if (!bun.Environment.isWindows) struct { ... } else struct { ... }- In the Unix branch, lines 1839-1840 declare
AddEnvironmentCleanupHookandRemoveEnvironmentCleanupHookwith their Itanium-mangled names - In the Windows branch, lines 1926-1927 declare the same functions with their MSVC-mangled names
- At lines 2487-2488,
inline for (comptime std.meta.declarations(V8API)) |decl| { std.mem.doNotOptimizeAway(&@field(V8API, decl.name)); }ensures all V8API symbols are retained - The five new async hooks symbols (e.g.,
_ZN4node29AsyncHooksGetExecutionAsyncIdEPN2v87IsolateEfor Unix) have no entries in either branch of the V8API struct - Searching
napi.zigforAsyncHooksorEmitAsyncreturns zero matches, confirming the omission
How to fix
Add the five mangled symbol names to both the Unix and Windows branches of the V8API struct in src/napi/napi.zig, following the same pattern as the existing AddEnvironmentCleanupHook/RemoveEnvironmentCleanupHook entries. The Unix (Itanium) mangled names are in symbols.txt and the Windows (MSVC) mangled names are in symbols.def.
1 participant
Summary
node::EmitAsyncInit,node::EmitAsyncDestroy,node::AsyncHooksGetExecutionAsyncId, andnode::AsyncHooksGetTriggerAsyncIdin Bun's V8 compatibility layerlibpqused bypg-native) reference these C++ API functions and fail to load without themCloses #28131
Test plan
test_v8_async_hookstest in the V8 test module (test/v8/v8-module/main.cpp) that calls all new functionscheckSameOutput(identical behavior under both Bun and Node.js)bun-debugbinary usingnm -Dtest_handle_scope_gcfailure unrelated to this change)🤖 Generated with Claude Code