[robobun]

Fixes #12918, fixes #32227, fixes #29194

Problem

• A program that replaces process.emit never gets 'beforeExit' or 'exit'. signal-exit replaces it, so onExit() does not run at a natural exit.
• dispatchExitInternal and Process__dispatchOnBeforeExit (src/jsc/bindings/BunProcess.cpp) emit on the native emitter. Node calls process.emit(event, code).

Fix

• Both sites read process.emit for each event. The built-in emit still runs natively.
• A replaced emit gets (event, code). The microtask checkpoint runs after it at 'beforeExit'.
• As in node, a natural exit skips a non-callable emit, and the main thread reports a throw as uncaught. process.exit() throws TypeError: process.emit is not a function, or passes the throw to its caller.
• Verified: 29 new cases in test/js/node/process/process.test.js, pinned to node v26.3.0 (25 fail on main). Also worker_threads.test.ts and 235 node parallel tests. Self-reviewed: 11 concerns raised, 11 addressed, two proposals not adopted (Notes).

Background

• process is a native EventEmitter. The runtime called the C++ emitter below process.emit.
• Considered: always call emit through JS (+1 VM entry, +2 allocations per exit for every program), and an own-property check (misses a prototype emit).

Downsides

• Never-replaced emit: +421 instructions per 'beforeExit' round, +460 at 'exit', no allocation. .text grows 1,280 bytes.
• A replaced emit now runs at exit and reaches two open defects. onExit(code => process.exit(code)) after an unhandled rejection exits 0, where main exits 1 (process: 'exit' listeners see exit code 1 after an unhandled rejection #45575). Jobs left by an 'unhandledRejection' listener run at 'beforeExit' (Run the ticks and microtasks that 'unhandledRejection' listeners queue #45597).
• A replaced emit that forwards to require("events").prototype.emit no longer reaches the listeners. 'warning', the uncaught-exception events, signals and IPC still bypass emit (Notes).

Notes

Result against node v26.3.0

• The four scripts of signal-exit package misbehaves with Bun (node compatibility) #12918 (onExit1.mjs, onExit2.mjs, writeFileAtomically1.mjs, writeFileAtomically2.mjs) with signal-exit 4.1.0 and write-file-atomic 7.0.1 print the same lines on node and on this branch. Main does not print onExit called with exit code 0 for the first two.
• The same holds for nine more scripts: signal-exit 3.0.7, both versions loaded together, { alwaysLast: true }, onExit after an uncaught error (onExit 1 null), a removed handler, and the repros of signal-exit onExit callback not triggered in Bun during process shutdown #29194 and process lifecycle events (beforeExit/exit) are not emitted via process.emit as in Node.js #32227.
• npm lockfile 1.0.4 uses signal-exit to remove its lock file. After lockSync() and a natural exit, node and this branch remove the file. Main leaves it on disk.
• Node's test suite has no test that replaces process.emit, so there is no upstream test to port. The 29 new scripts are bun -e children. 26 of them print the same lines and exit with the same code on node. The other 3 say in a comment what node does.

How the lookup works

• replacedProcessEmit first looks, without a [[Get]], for the common state: process has no own emit and its prototype holds the built-in function. JSEventEmitter::isBuiltInEmit compares the host function pointer.
• If that fails, it does the full process.emit read that node does. An emit on the prototype, on an inserted prototype, or behind an accessor is found. An accessor is read once for each event.
• If the value is the built-in emit again (signal-exit puts it back when its last handler is removed), the native path runs.

Answers to the review comment of 2026-09-23

1. Own property only: the read is now the full one, see above.
2. Non-callable emit: a natural exit emits nothing and throws nothing. process.exit() throws the TypeError.
3. The checkpoint after 'beforeExit': it is not gated on the value that emit returns. Node runs the ticks and microtasks after the call whatever it returned, and the case "the ticks and microtasks that emit queues run before 'exit', whatever emit returns" pins that. The error: kaboom of test/js/node/test/parallel/test-event-capture-rejections.js with a replaced emit preloaded has another cause. On main that test runs 3 of its 13 stages and exits 0, because a tick that an 'unhandledRejection' listener queues last never runs. A preload of only process.on("beforeExit", () => {}) gives the same error: kaboom on main. Run the ticks and microtasks that 'unhandledRejection' listeners queue #45597 corrects both defects behind it (Run a microtask checkpoint after each batch of unhandledRejection listeners, and follow Node's captureRejections #42031 is closed in its favor). With no preload the test passes here as on main.
4. The branch has current main merged.

One exit latch

• An earlier commit on this branch gave process.exit() its own latch, so that the exit after a process.exit() that threw emits 'exit' again, as node does. That ran the 'exit' listeners twice when an 'exit' listener threw after process.exit(), in programs that never replace emit. The latch is the one main has again (m_isExiting), and two cases guard it.
• What stays different from node: after a process.exit() whose emit threw (or was not callable), the exit that follows emits no 'exit'.

Measurements

Release bun-profile, Linux x64, main bd599f5af against this PR at e38058be8. Instructions of the main thread, counted with gdb stepi from the entry of Process__dispatchOnBeforeExit to its return, and of VirtualMachine::on_exit to its return (LTO inlines Process__dispatchOnExit there). Counts repeat exactly unless the row says otherwise.

script	'beforeExit' main	PR	'exit' main	PR
1 + 1 (no process object)	4	17	136	132
process.exitCode = 0	540	961	934	1,394
built-in emit put back as an own property	540	1,055	934	1,488
signal-exit onExit(), then removed	540	1,055	775	1,329
process.exitCode = 0; Bun.gc(true)	540	10,908	10,792	1,394
one listener for each event (runs script, not repeatable)	12,362 to 12,922	13,185 to 15,682	11,773 to 14,120	11,772 to 17,872

• After a GC, the first property lookup on process rebuilds its property table (about 10,000 instructions, one allocation). Main does that at 'exit', where it writes _exiting. This PR does it at 'beforeExit'. The sum of both events grows by about 950.
• Calls inside both regions for the first four rows: mi_malloc and mi_free 1 and 1 on both builds, no entry into JS, no call of the built-in emit host function, no checkpoint.
• With a replaced emit (the pass-through of process lifecycle events (beforeExit/exit) are not emitted via process.emit as in Node.js #32227): 1 entry into JS and 1 JSString for each event, and 1 checkpoint for each 'beforeExit'. The first 'exit' costs 7,746 instructions. The first 'beforeExit' compiles the function and is not repeatable.
• .text: 65,453,397 to 65,454,677 bytes (+1,280). The stripped bun is 88,938,056 bytes on both builds.
• Process::finishCreation is identical (2,413 bytes). No new builtin name. sizeof(Bun::Process) is 136 on both.
• BUN_JSC_validateExceptionChecks=1: no report for a throwing emit, a throwing getter and a non-callable emit, each at a natural exit and in process.exit().

Differences from node that stay

• A throw at 'exit' sets the exit code to 1. Node keeps the code that was set. Two cases pin the 1 and say so. Report a loop turn's rejections before the loop-alive check, and keep the exit code when an 'exit' listener throws #42032 covers it.
• A microtask that 'exit' queues does not run. Node runs it after the 'exit' emit (and drops the ticks, because nextTick is a no-op while _exiting is set). Main does not run it for a native listener either, and a replaced emit matches the native path here.
• After an unhandled rejection, 'exit' gets the code 0 and the process exits with 1 (process: 'exit' listeners see exit code 1 after an unhandled rejection #45575, bun run: decide the exit code of a printed error before 'exit' listeners run #38116). This is also what a plain 'exit' listener gets on main.
• In a worker, a throw from emit (or from a listener) at the worker's 'exit' does not reach the parent as an 'error' event (Report a loop turn's rejections before the loop-alive check, and keep the exit code when an 'exit' listener throws #42032).
• An accessor emit whose getter throws: the process exits with 1. Node exits with 7, because its fatal-exception path reads emit again.
• process keeps its listeners in the native emitter. A patch on require("events").prototype.emit is not seen, and a replaced emit that forwards to that function reaches no listener. On main such a program still got its 'beforeExit' and 'exit' listeners, because the runtime did not call emit.
• Bun's own 'exit' listeners (trace events, worker stdio, process.finalization) are listeners like the others. A replaced emit that does not forward 'exit' skips them.

Events that still bypass a replaced emit

• 'warning' (jsFunction_emitWarning). The pattern that hides one warning with a replaced emit still prints the warning. The two helpers of this PR fit that site.
• 'uncaughtExceptionMonitor', 'uncaughtException', 'unhandledRejection', 'rejectionHandled'.
• Signal events. Node binds emit when the first listener of a signal is added.
• 'message', 'internalMessage' and 'disconnect' (fix(process): dispatch child IPC through JavaScript emit #44546), 'memoryPressure', 'error', 'newListener' and 'workerMessage'.

Suites, debug build, this branch merged into main 5c6d4e24cd (2026-10-23)

• The merge has no conflict. The new block: 29 pass. With src/ of main, 25 of them fail. The 4 that pass on main guard existing behavior. The two worker cases run the worker in the test process and read its events from a SharedArrayBuffer, under 1 s each in a debug build.
• Node v26.12.1 prints the same lines and exits with the same codes as v26.3.0 for the 27 subprocess cases.
• test/js/node/process/process.test.js: 225 pass. 1 fails on a host that has node v26.3.0, because main wants v26.12.1 there (should be the node version on the host that we expect).
• test/js/node/worker_threads/worker_threads.test.ts: 161 pass.
• test/js/node/test/parallel: 235 files (test-process-*, test-worker-*, test-next-tick*, test-promise-unhandled*, and the exit, signal and uncaught-exception tests). All pass. An earlier run on this branch reported 215 of 220.

The two defects of Downsides, with the open PRs on top (2026-10-23)

• onExit(code => process.exit(code)) after an unhandled rejection: the handler gets 0 and the process exits with 0. On main the handler does not run and the process exits with 1. With the run_command.rs change of process: 'exit' listeners see exit code 1 after an unhandled rejection #45575 applied on top, the handler gets 1 and the process exits with 1, as on node.
• test-event-capture-rejections.js with a pass-through emit preloaded: error: kaboom. With Run the ticks and microtasks that 'unhandledRejection' listeners queue #45597 merged on top, it exits 0. It also exits 0 there with only a 'beforeExit' listener preloaded.

Self-review

• 11 concerns. The main ones: a second exit latch (removed, see above), a throw at 'beforeExit' that a handler took and that skipped the checkpoint (fixed), the shape of the helper (one helper for any event, and the 'worker' helper is untouched), missing cases (added), and downsides that the description did not name (added).
• Not adopted: new issues for the defects that no user reported. They are listed above with the open PRs that cover them.
• Not adopted: a rebase onto Run a microtask checkpoint after each batch of unhandledRejection listeners, and follow Node's captureRejections #42031 (closed since, in favor of Run the ticks and microtasks that 'unhandledRejection' listeners queue #45597). This PR does not need it to be correct.

An earlier version of this change was #29196.

---

no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/process/process.test.js

[robobun]

^{Updated 3:31 AM PT - Nov 8th, 2026}

✅ @robobun, your commit a180822528a09f74bcd624cd0a1a85cadebe42e7 passed in Build #138565! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 32228

That installs a local version of the PR into your bun-32228 executable, so you can run:

bun-32228 --bun

[github-actions]

Found 3 issues this PR may fix:

1. signal-exit package misbehaves with Bun (node compatibility) #12918 - signal-exit package monkey-patches process.emit to observe exit events; this PR routes exit/beforeExit through JS-visible process.emit, which is the core fix signal-exit needs
2. process.beforeExit event is not emitted during normal shutdown #30934 - process.beforeExit not emitted during normal shutdown; directly addressed by this PR's changes to emit beforeExit through process.emit
3. process.on("exit") isn't triggered with bun test #8434 - process.on("exit") not triggered with bun test; likely shares the same exit dispatch path fixed here (removing the early-return when no listeners are registered)

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #12918
Fixes #30934
Fixes #8434

🤖 Generated with Claude Code

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

• Last reviewed commit: 9d60825
• Latest commit, not reviewed: a180822

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Process lifecycle events (beforeExit, exit) are dispatched through the JS-visible process.emit via a new helper that resolves and calls process.emit (falls back to internal emitter). Native dispatch runs the helper under a top exception scope, reports/clears thrown exceptions, and tests validate monkey-patched process.emit behavior via subprocesses.

Changes

Process lifecycle event dispatch via process.emit

Layer / File(s)	Summary
Emit helper and native dispatch updates src/jsc/bindings/BunProcess.cpp	Add emitProcessExitEvent that calls the JS-visible process.emit (resolving accessors) or falls back to the internal emitter, returns a boolean, throws on non-callable overrides, and update dispatchExitInternal and Process__dispatchOnBeforeExit to call the helper under a top exception scope and report/clear exceptions.
Subprocess tests for monkey-patched process.emit test/js/node/process/process.test.js	Add subprocess-based tests that monkey-patch process.emit to verify beforeExit and exit are invoked with correct code argument, that patched emit composes with process.on('exit'), forwards process.exitCode and explicit process.exit(code), and that process.nextTick scheduled from a patched process.emit runs before exit.

Pre-merge checks: 4

Check name	Status	Explanation
Linked Issues check		Code changes implement all objectives from #32227 and #29194: routing lifecycle events through JS-level process.emit so user-overridden emit and monkey-patch libraries observe beforeExit/exit events.
Out of Scope Changes check		All changes directly address the lifecycle emission problem: the C++ refactoring adds emitProcessExitEvent helper, and tests validate the fix with subprocess spawning to confirm monkey-patch observabi…
Title check		The title clearly and concisely describes the main change: routing the beforeExit and exit lifecycle events through a replaced process.emit.
Description check		The description provides the problem, fix, verification results, background, trade-offs, test coverage, and known differences. Although it does not use the template headings exactly, it includes the r…

Full details: Out of Scope Changes check

Explanation

All changes directly address the lifecycle emission problem: the C++ refactoring adds emitProcessExitEvent helper, and tests validate the fix with subprocess spawning to confirm monkey-patch observability.

Full details: Description check

Explanation

The description provides the problem, fix, verification results, background, trade-offs, test coverage, and known differences. Although it does not use the template headings exactly, it includes the required information and is substantially complete.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/jsc/bindings/BunProcess.cpp (1)

856-873: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't snapshot beforeExit listeners before calling overridden process.emit.

hadListeners is captured before JS runs, but the whole point of this change is that process.emit is now user-overridable. An override can add the first beforeExit listener and then delegate to the original emit; that listener will fire and may queue nextTick work, but Lines 869-873 will still skip draining because the stale pre-call snapshot stayed false. Base the drain decision on the actual process.emit result instead of the pre-call wrapped() state.

Suggested direction

-static void emitProcessExitEvent(JSC::JSGlobalObject* globalObject, Process* process, ASCIILiteral eventName, int exitCode)
+static bool emitProcessExitEvent(JSC::JSGlobalObject* globalObject, Process* process, ASCIILiteral eventName, int exitCode)
 {
     auto& vm = JSC::getVM(globalObject);
     auto scope = DECLARE_THROW_SCOPE(vm);
@@
-    JSC::call(globalObject, emitValue, callData, process, arguments);
-    RETURN_IF_EXCEPTION(scope, );
+    JSValue didEmit = JSC::call(globalObject, emitValue, callData, process, arguments);
+    RETURN_IF_EXCEPTION(scope, false);
+    return didEmit.toBoolean(globalObject);
 }
@@
-    bool hadListeners = process->wrapped().hasEventListeners(Identifier::fromString(vm, "beforeExit"_s));
-
     auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
-    emitProcessExitEvent(globalObject, process, "beforeExit"_s, exitCode);
+    bool hadListeners = emitProcessExitEvent(globalObject, process, "beforeExit"_s, exitCode);
     if (auto* exception = scope.exception()) [[unlikely]] {
         (void)scope.tryClearException();
         Bun__reportUnhandledError(globalObject, JSValue::encode(exception));
         return;
     }

🤖 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/BunProcess.cpp` around lines 856 - 873, The code captures
hadListeners before calling emitProcessExitEvent, but because process.emit can
be overridden the decision to drain nextTickQueue must be based on the actual
post-emit state; remove the pre-call snapshot (hadListeners) and instead either
(A) read the listener state after emit by calling
process->wrapped().hasEventListeners(Identifier::fromString(vm, "beforeExit"_s))
and use that result to decide whether to drain nextTickQueue, or (B) change
emitProcessExitEvent to return a bool indicating whether the emit had listeners
and use that return value to gate nextTickQueue->drain; update the code around
emitProcessExitEvent, hadListeners, and nextTickQueue->drain accordingly
(referencing hadListeners, emitProcessExitEvent,
process->wrapped().hasEventListeners, and nextTickQueue->drain).

🤖 Prompt for all review comments with 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.

Inline comments:
In `@src/jsc/bindings/BunProcess.cpp`:
- Around line 292-294: The current check after calling
JSC::getCallData(emitValue) silently returns when callData.type ==
CallData::Type::None; instead, mirror Process_functionEmitHelper by creating a
pending "not a function" exception so the caller can surface it during shutdown:
when callData.type == CallData::Type::None, invoke the same
createNotAFunctionError path used by Process_functionEmitHelper (i.e.,
produce/throw the not-a-function exception for emitValue) rather than returning
silently, leaving the pending exception for the caller to report.
- Line 289: The code creates a new Identifier for "emit" on each lifecycle
dispatch by calling Identifier::fromString(vm, "emit"_s); replace those
constructions with the cached name vm.propertyNames().emit (the same cached
property used in processGet) so that process->get(globalObject,
Identifier::fromString(vm, "emit"_s)) becomes process->get(globalObject,
vm.propertyNames().emit) (apply the same replacement at all beforeExit/exit
dispatch sites in BunProcess.cpp to avoid per-dispatch allocations).

---

Outside diff comments:
In `@src/jsc/bindings/BunProcess.cpp`:
- Around line 856-873: The code captures hadListeners before calling
emitProcessExitEvent, but because process.emit can be overridden the decision to
drain nextTickQueue must be based on the actual post-emit state; remove the
pre-call snapshot (hadListeners) and instead either (A) read the listener state
after emit by calling
process->wrapped().hasEventListeners(Identifier::fromString(vm, "beforeExit"_s))
and use that result to decide whether to drain nextTickQueue, or (B) change
emitProcessExitEvent to return a bool indicating whether the emit had listeners
and use that return value to gate nextTickQueue->drain; update the code around
emitProcessExitEvent, hadListeners, and nextTickQueue->drain accordingly
(referencing hadListeners, emitProcessExitEvent,
process->wrapped().hasEventListeners, and nextTickQueue->drain).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 90c4acc7-f07f-43af-beb0-b6230ed02e0a

📥 Commits

Reviewing files that changed from the base of the PR and between a0e221e and 54623ba.

📒 Files selected for processing (2)

• src/jsc/bindings/BunProcess.cpp
• test/js/node/process/process.test.js

[github-actions]

This PR may be a duplicate of:

1. runtime: exit on unsettled top-level await instead of spinning (Ctrl+C at @inquirer/prompts) #30601 - Also fixes signal-exit onExit callback not triggered in Bun during process shutdown #29194 by routing beforeExit/exit through JS-level process.emit instead of the internal C++ EventEmitter, addressing the same process.emit monkey-patching bypass

🤖 Generated with Claude Code

[robobun]

I checked the three suggested issues against their repros on a current build before adding any Fixes lines:

• process.beforeExit event is not emitted during normal shutdown #30934 (beforeExit not emitted during normal shutdown): already fixed on 1.4.0. bun -e "process.on('beforeExit', () => console.log('hi'))" prints hi. Not changed by this PR.
• process.on("exit") isn't triggered with bun test #8434 (process.on("exit") not firing under bun test): separate path. The bun test runner shutdown does not go through this process exit dispatch, and the exit listener still does not fire with this change, so it is not fixed here.
• signal-exit package misbehaves with Bun (node compatibility) #12918 (signal-exit): this PR supplies the process.emit routing signal-exit needs on normal exit (same root cause as signal-exit onExit callback not triggered in Bun during process shutdown #29194, already in the Fixes list). The issue also covers SIGINT scenarios this focused change does not touch, so I am leaving it off the Fixes list.

Keeping Fixes #32227 and Fixes #29194, both verified by the spawned tests.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with 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.

Inline comments:
In `@src/jsc/bindings/BunProcess.cpp`:
- Around line 880-887: The call to JSNextTickQueue::drain(vm, globalObject) can
enter JS and leave a pending exception, so after the call in BunProcess.cpp add
the standard JSC binding exception check: wrap the call site with a ThrowScope
(or ensure a ThrowScope is active) and invoke RETURN_IF_EXCEPTION(vm, ... ) /
RETURN_IF_EXCEPTION(vm) as per project convention immediately after
nextTickQueue->drain(vm, globalObject); so any pending exception is detected and
returned (matching the existing handling used for
emitProcessExitEvent/Process__dispatchOnBeforeExit).
- Around line 316-318: The wrapper currently uses the patched emit's return
value directly (JSValue result in BunProcess.cpp) which lets a monkey-patched
process.emit returning undefined/false suppress draining; change the return
logic so only an actual boolean false from the emit prevents draining: after
calling emitOverride in the function that dispatches beforeExit, check
result.isBoolean() and if true return result.toBoolean(globalObject), otherwise
return true (so undefined/non-boolean values do not block
m_nextTickQueue->drain()); update the code around the JSValue result /
RETURN_IF_EXCEPTION call to implement this boolean-only interpretation.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 70ea78d8-4fa1-4dea-887e-ba4fa3565f4b

📥 Commits

Reviewing files that changed from the base of the PR and between 54623ba and 4e8c9ef.

📒 Files selected for processing (2)

• src/jsc/bindings/BunProcess.cpp
• test/js/node/process/process.test.js

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/jsc/bindings/BunProcess.cpp (1)

300-321: ⚠️ Potential issue | 🟠 Major

Fix accessor process.emit overrides: avoid getDirect
emitProcessExitEvent reads process.emit via process->getDirect(..., "emit"_s), which bypasses JS accessor/getter invocation; if process.emit is an accessor own-property, this can treat the stored GetterSetter as “not a function” instead of calling the getter result. Keep the own-property detection, but fetch the override through normal JS property access semantics on the override branch, then JSC::call.

🤖 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/BunProcess.cpp` around lines 300 - 321, The code uses
process->getDirect to read process.emit which bypasses JS accessor getters;
instead, keep the own-property detection but fetch the override with normal
property access so getters run: check ownership (e.g. process->hasOwnProperty or
getOwnProperty) for "emit" to decide whether to use the override branch, and
when you need the override value use process->get(globalObject,
Identifier::fromString(vm, "emit"_s)) (not getDirect) to obtain emitOverride,
then use JSC::getCallData/call as before (preserve process->wrapped().emit
fallback logic and the call error handling).

🤖 Prompt for all review comments with 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.

Outside diff comments:
In `@src/jsc/bindings/BunProcess.cpp`:
- Around line 300-321: The code uses process->getDirect to read process.emit
which bypasses JS accessor getters; instead, keep the own-property detection but
fetch the override with normal property access so getters run: check ownership
(e.g. process->hasOwnProperty or getOwnProperty) for "emit" to decide whether to
use the override branch, and when you need the override value use
process->get(globalObject, Identifier::fromString(vm, "emit"_s)) (not getDirect)
to obtain emitOverride, then use JSC::getCallData/call as before (preserve
process->wrapped().emit fallback logic and the call error handling).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b6b062c8-add6-42f7-bf96-42f9651e6b59

📥 Commits

Reviewing files that changed from the base of the PR and between 740f807 and 9d60825.

📒 Files selected for processing (1)

• src/jsc/bindings/BunProcess.cpp

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with 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.

Inline comments:
In `@src/jsc/bindings/BunProcess.cpp`:
- Around line 300-327: The current logic only calls process->getDirect(...) and
ignores prototype-level overrides; change the resolution so you perform a full
property lookup (process->get(...)) whenever getDirect() returns no own property
(i.e., emitOverride is empty), or when getDirect() yields an accessor
(isGetterSetter/isCustomGetterSetter) — but keep the current behavior that if
getDirect() returns a non-callable own property you fall back to the internal
emitter (wrapped().emit). Concretely: call process->getDirect(...) into
emitOverride; if emitOverride is empty OR
emitOverride.isGetterSetter()/isCustomGetterSetter() then set emitOverride =
process->get(globalObject, Identifier::fromString(vm, "emit"_s)) and check
RETURN_IF_EXCEPTION; then obtain callData from emitOverride and if callable
invoke JSC::call(...) as currently done; only when the original getDirect()
returned a non-callable own property should you skip the prototype lookup and
fall through to wrapped().emit(Identifier::fromString(vm, eventName),
arguments). Ensure you reference emitOverride, process->getDirect, process->get,
callData, and wrapped().emit when applying the change.

In `@test/js/node/process/process.test.js`:
- Around line 658-685: Update the test "invokes a process.emit override defined
as an accessor property" to exercise the accessor override for both lifecycle
events: beforeExit and exit. Specifically, in the inline script that defines
Object.defineProperty(process, "emit", ...) (the accessor-based override of
process.emit), make the getter-returned function detect both "beforeExit" and
"exit" and log distinct markers (e.g., "accessor:beforeExit" and
"accessor:exit"), then update the test assertions that inspect stdout/exitCode
to expect both markers; this ensures the accessor [[Get]] path is covered for
process.emit when invoked for beforeExit as well as exit.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: eac4eec1-8675-4ec5-aa22-6c81ea54aa65

📥 Commits

Reviewing files that changed from the base of the PR and between 9d60825 and ee8abaf.

📒 Files selected for processing (2)

• src/jsc/bindings/BunProcess.cpp
• test/js/node/process/process.test.js

[claude]

All my earlier feedback has been addressed and the bug hunter found nothing new on f4efbd2, but this reworks process shutdown dispatch in the JSC bindings and went through several design iterations (getDirect vs full [[Get]], drain-timing, non-callable fallback) — worth a human sign-off on the final trade-offs.

Extended reasoning...

Overview

This PR routes the beforeExit and exit lifecycle events through a user-replaceable process.emit so monkey-patches (signal-exit etc.) observe them, matching Node. It adds a ~50-line emitProcessExitEvent helper in src/jsc/bindings/BunProcess.cpp, rewires dispatchExitInternal and Process__dispatchOnBeforeExit to use it under a top exception scope, removes the exit early-return on zero listeners, and adds six subprocess tests in test/js/node/process/process.test.js.

Security risks

None identified. The change does not touch auth, crypto, permissions, or untrusted input parsing. It changes which JS function is invoked at shutdown, but that function is already user-controllable (it's process.emit); Node already behaves this way.

Level of scrutiny

High. This is core process-lifecycle dispatch in C++ JSC bindings — every Bun process runs this code on shutdown. The PR went through six substantive revisions during review, each fixing a real regression caught by CI or reviewers: an ASAN unchecked-exception abort after drain(), a worker_threads.test.ts regression from throwing on non-callable emit, and a test-event-capture-rejections.js drain-timing regression from using a full [[Get]]. The final design encodes deliberate, non-obvious trade-offs: getDirect (own-property only) instead of full [[Get]] so the no-override path keeps the native "a listener fired" signal that gates the post-beforeExit drain; accessor overrides resolved with a targeted full [[Get]]; non-callable overrides falling through to the internal emitter rather than throwing; prototype-level emit replacements intentionally not honored. These are well-reasoned and documented in code comments and the PR description, but they are semantic judgment calls a maintainer should ratify.

Other factors

All seven of my prior inline comments across three review rounds are resolved, and the bug-hunting system found nothing on the current head. The two CI failures on f4efbd2 (streams-leak, fetch-abort-slow-connect) are known flakes unrelated to this change, per the a348996 retrigger commit. Test coverage is solid (six new subprocess tests covering the override matrix plus existing worker_threads.test.ts for the non-callable fallback). I'm not approving because the change is neither simple nor mechanical and the iteration history shows the semantics here are easy to get subtly wrong.

[robobun]

Rebased onto current main (was conflicting after the process.on('memoryPressure') / Node 26.3.0 changes landed) and squashed the iteration history into a single commit. The only conflict was in dispatchExitInternal: main switched the exit re-entrancy guard from a static processIsExiting bool to the process->m_isExiting member, which I adopted (the exit emit still routes through emitProcessExitEvent). Rebuilt and reran the lifecycle tests, worker_threads non-callable coverage, and test-event-capture-rejections.js, all green.

[robobun]

CI status on e2ae14fa (build 67800): the process.emit lifecycle change is green. The only non-flaky failure is test/bake/dev/production.test.ts (bake dev-server SSR, e.g. "importing useState from client component" timing out at 5000ms), and a flaky install test that passed on retry. Neither is related to this PR: the diff is two files (BunProcess.cpp beforeExit/exit dispatch + process.test.js) with no path into the bake/SSR subsystem, and those bake tests time out the same way locally on a debug build. I have already used my one CI re-roll, so I am not pushing another retrigger. The branch is rebased onto current main, mergeable, with all review threads resolved; it needs a maintainer to re-run the bake lane (or confirm it as a pre-existing flake) and sign off on the final design.

[robobun]

Stale PR review: keep open, rework.

The fix is still wanted. Node v26.3.0 calls a replaced process.emit for beforeExit and exit. Bun on main does not: dispatchExitInternal and Process__dispatchOnBeforeExit call the internal emitter directly (src/jsc/bindings/BunProcess.cpp:347 and :862). That is the cause of the confirmed bug #12918 (signal-exit). I found no other open PR that covers it.

The current diff is not the shape to land:

• It reads process.emit with getDirect, so it sees an own property only. Node also sees an emit that a program patches on the prototype. Process_functionEmitHelper in the same file (BunProcess.cpp:4904) already does the full process->get(globalObject, "emit") lookup. Use that lookup.
• If process.emit is not callable (for example process.emit = 5), Node fires no listeners. This diff falls back to the internal emitter and fires them.
• It returns true after each call of the override, so the caller always drains the nextTick queue. Load a pass-through override, the shape signal-exit installs. A build of this PR then fails test/js/node/test/parallel/test-event-capture-rejections.js with error: kaboom. Node and main pass that test with the same preload. One option: gate the drain on the value that emit returns, as main does with fired.
• The branch conflicts with main. Rebase it.

[robobun]

Status: reworked on current main. The description is new.

How to reproduce #12918. Copy signal-exit 4.1.0 into a temporary node_modules and run this file:

import { onExit } from "signal-exit";
process.on("SIGINT", signal => console.log("Native signal handler got " + signal));
onExit(exitCode => console.log("onExit called with exit code " + exitCode));
setTimeout(() => console.log("bye!"), 600);
setTimeout(() => process.kill(process.pid, "SIGINT"), 300);

• Node v26.3.0 prints Native signal handler got SIGINT, bye!, onExit called with exit code 0.
• Main (bd599f5af) prints the first two lines.
• This branch prints all three. The four scripts of the issue give the same output as node.

What changed against the earlier version of this PR. The lookup is the full process.emit read, so an emit on the prototype is found. A non-callable emit emits nothing at a natural exit. The checkpoint after 'beforeExit' is not gated on the value that emit returns. The Notes of the description answer the review comment of 2026-09-23 point by point and list what stays different from node.

[claude]

Beyond the inline findings, two more candidates were checked and ruled out: the top exception scope that emitThroughReplacedEmit opens beneath Process_functionExit's throw scope leaves a throwing or non-callable emit pending for the caller's RETURN_IF_EXCEPTION rather than tripping the exception-check validator, and a non-callable process.emit skipping the native 'exit' listeners at a natural exit matches node, whose MakeCallback also emits nothing in that case.

Extended reasoning...

The change routes 'beforeExit' and 'exit' through a user-replaced process.emit in src/jsc/bindings/BunProcess.cpp, adds JSEventEmitter::isBuiltInEmit, and adds ~30 subprocess cases; it touches no security-sensitive surface. Seven inline findings were posted, so a human look is already signalled; this note only records the two further candidates examined and ruled out this run.

Findings marked 🟡 are optional suggestions and need no follow-up push.

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline test finding, I also checked the two refactor hazards in src/jsc/bindings/BunProcess.cpp: Process_functionExit calling dispatchExitInternal directly only skips the hasProcessObject guard, which is already true there (process is dereferenced on the line above), and the m_isExitCodeObservable latch now runs before the m_isExiting early return exactly as it did in Process__dispatchOnExit. The tryClearException in emitThroughReplacedEmit is under its own DECLARE_TOP_EXCEPTION_SCOPE and hands the exception to Bun__reportUnhandledError only on the MakeCallback shape, so nothing is swallowed and the ProcessExit shape still propagates to the caller.

Extended reasoning...

The change routes 'beforeExit' and 'exit' through a user-replaced process.emit in native process-lifecycle code (BunProcess.cpp, JSEventEmitter) and adds ~30 spawned test cases; it touches no auth, crypto, or input-parsing surface. A human look is still warranted because the exit-path semantics (latch ordering, exception reporting, microtask drain) are subtle and one test-wiring finding remains inline.

[claude]

I re-reviewed this push and found no new bugs; the two test-only commits since my last review close the nits I had open (pinned exit codes, no per-test timeout, Worker error wired to reject). A maintainer should still weigh the documented tradeoffs before merging, since the C++ path is unchanged and the PR deliberately accepts some divergences from node.

What was reviewed:

• replacedProcessEmit fast path: own-property getDirect falls through to a full [[Get]] for accessors, and the prototype check is gated on overridesGetOwnPropertySlot, so Proxy prototypes take the slow path.
• Exception flow in emitThroughReplacedEmit: MakeCallback shape clears and reports, ProcessExit shape leaves the throw pending for Process_functionExit's RETURN_IF_EXCEPTION; the beforeExit site drains microtasks then RELEASE_AND_RETURNs.
• The moved m_isExitCodeObservable latch: now set before the m_isExiting early return, which is the same ordering Process__dispatchOnExit had for the natural-exit path.
• Test block: -e spawns drain stdout/stderr/exited together and assert a combined object; the in-process worker cases now reject on error.

Extended reasoning...

The change routes 'beforeExit' and 'exit' through a user-replaced process.emit in src/jsc/bindings/BunProcess.cpp, adds JSEventEmitter::isBuiltInEmit, and rewires process.exit() to call dispatchExitInternal directly; it touches no auth, crypto, or injection surface. It is a behavior change to process lifecycle with several documented divergences from node (forwarders through EventEmitter.prototype.emit lose listeners, exit code 1 on a throw at 'exit', no microtask drain after 'exit'), which the author has chosen to accept rather than fix here. The C++ has not changed since my prior inline findings, which the author self-resolved with replies and by documenting the tradeoffs in the description, so a human maintainer should make that product call; the test-only follow-ups fully addressed my remaining nits.

[robobun]

I checked this branch against main at 5c6d4e2. That is 189 commits after the merge base, with five WebKit bumps and the update to Node.js 26.12.1.

• The merge has no conflict and builds (Linux x64, debug with ASAN). I did not push it. Build #127872 stays the CI result for 14fb03c.
• The 29 cases of this PR pass on the merged tree. With src/ of main, 25 fail.
• Node v26.12.1 gives the same result as v26.3.0 for the 27 subprocess cases.
• process.test.js: 225 pass, and 1 fails because this host has node v26.3.0. worker_threads.test.ts: 161 pass.
• 235 node parallel files (process, workers, ticks, exit, signals, uncaught exceptions): all pass.

Merge order. After an unhandled rejection, a signal-exit handler that calls process.exit(code) gets the code 0 with this PR. The process then exits with 0. On main that handler does not run and the process exits with 1. I applied the run_command.rs change of #45575 on top of this branch. The handler then gets 1 and the process exits with 1, as on node. If #45575 lands first, that case keeps its exit code.

The description named #42031, which is closed in favor of #45597. It now names #45597. With #45597 merged on top, test-event-capture-rejections.js exits 0 with a pass-through emit preloaded.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[robobun]

Merged main (2596ac5) into this branch. One conflict, in src/jsc/bindings/BunProcess.cpp.

• main added runWhatRunsWhenProcessExits() in front of dispatchExitInternal(), where this PR adds its helpers. Both are kept.
• dispatchExitInternal() runs the hook of main first. Then it emits exit as this PR does.
• Verified on a debug build of the merge: test/js/node/process/process.test.js, 226 pass. The one failure, should be the node version on the host that we expect, needs Node 26.12.1 on the host. This machine has 26.3.0.