process.exit(): throw process.reallyExit is not a function, pass process as this - #44057
Merged
Jarred-Sumner merged 1 commit intoSep 27, 2026
Merged
Conversation
…en it is not callable process.exit() looks up process.reallyExit and calls it. The call passed an empty string as the message for a value that is not callable, so the TypeError had no message, and a build with assertions aborted on `!message.isEmpty()` in JSC::createTypeError. The call also passed the function itself as the this value. The call now passes process as the this value and throws `TypeError: process.reallyExit is not a function`, as Node.js does.
Collaborator
Author
|
Status: the fix and the tests are pushed. How to reproduce: // repro.js
process.reallyExit = "str";
process.exit(0);
On this branch, |
Contributor
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: oven-sh/bun/.coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review. Walkthrough
ChangesProcess exit behavior
Suggested reviewers: Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to No merge-blocking issue is established for the receiver-binding and error-message change. Merge after normal checks. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Jarred-Sumner
deleted the
robobun/c0daf32a/process-exit-reallyexit-not-callable
branch
September 27, 2026 00:46
Seantheprogrammer93
pushed a commit
to Seantheprogrammer93/bun
that referenced
this pull request
Sep 27, 2026
…h#42116) ### Problem - `new Response(p.body, p)` adopted a JS-backed stream but pulled the Blob out of a blob-backed one (`Value::from_js`): `p` came out locked and used, and `r.body !== p.body`. - After `text()`/`json()`/`blob()`/... the body's stream was unlocked and `getReader()` worked. undici and Chromium keep it locked. - Zero-length string/`Uint8Array` bodies were never used up: `blob()` left `bodyUsed` false, and `.body` handed out a stream the body did not track. ### Fix - `Value::from_js` adopts every stream. `to_blob_if_possible` still lifts a blob/file-backed stream back into a blob when the body is consumed, served, or uploaded, so the Content-Length framing stays. - New `m_consumedAsBody` bit on `JSReadableStream`, part of `nativeHandleDetached()` and so of `isReadableStreamLocked()`. `set_promise` sets it once the consumer has started, `to_any_blob` after taking a native source's bytes. `Bun.readableStreamToText()` is unchanged. - `.body` on `Empty` stores its stream like the other arms, `use_()` marks `Empty` used, `to_any_blob` turns a closed never-read stream into an empty blob, the getters reject a locked stream up front, and `Response.redirect()`/`error()` get a null body. - Verified: `test/js/web/fetch/body.test.ts` (145 new cases, 128 fail on 1.4.3, all checked against node v26.3.0), plus the suites in Notes. Self-reviewed: 4 concerns, 3 addressed, the oven-sh#33461 overlap is noted below. ### Background - A body is a `Body::Value`: string, `Blob`, bytes, `Empty`, `Null`, or `Locked` (a stream). Reading `.body` makes a non-null body `Locked`. - Blob- and file-backed streams keep a native source. Until something reads them, `to_any_blob` can take the payload back without running the stream. The fetch spec reads a body through a reader it never releases, so a consumed body's stream stays disturbed and locked. <details><summary>Notes</summary> - Ledger members: oven-sh#44053 (eager transfer), oven-sh#44054 and oven-sh#44171 (unlocked after consume, JS-stream close and error paths), oven-sh#44055 (zero-length bodies). Not in this PR: oven-sh#44057 is covered by oven-sh#33499, and the "`getReader()` alone marks a native body used" cascade (oven-sh#921) by oven-sh#33461. oven-sh#44059, oven-sh#44060 and oven-sh#44061 are separate mechanisms. - Overlap with oven-sh#33461: both touch the body getter prologues, `ReadableStream::to_any_blob`'s guard and `Value::from_js`. If this lands first, oven-sh#33461 keeps its `m_nativeSourceMaterialized` gating and drops its getter and `from_js` hunks on rebase. `to_any_blob` then wants `is_native_source_consumed || is_locked` as its guard. - `ReadableStream__detach`/`force_detach` had no other caller and is removed. `m_consumedAsBody` takes over both halves of what the `-1` handle sentinel did there: the stream reads as locked, and its native handle is neither started by `getReader()` nor handed to `Readable.fromWeb()`'s fast path. Unlike the sentinel it leaves `m_nativePtr` alone, so the handle stays rooted while an async consumer runs. - `Readable.fromWeb()` now throws `ERR_INVALID_STATE` for any locked stream before it does anything else, as Node does (Node acquires the reader at that point). Before, a locked native-backed stream had its handle taken anyway. - `ReadableStream__isClosedUnread`: `ReadableStream{Default,Byte}ControllerClose` only moves a stream to `Closed` once its queue is empty, so `Closed && !disturbed && !locked` means the stream can never yield a byte. This keeps a touched empty body (`new Response(""); r.body`) framing and typing exactly like an untouched one, and `new Response(new Blob([]))` takes the same path. - A locked (not disturbed) body stream now rejects from the getter with `TypeError: Invalid state: ReadableStream is locked`, the same error the C++ helper produced before, and no longer records a pending read first. For JS-stream bodies `getReader(); releaseLock(); await r.text()` works and `bodyUsed` stays false while only locked, as in undici. Native-backed bodies still mark themselves disturbed on `getReader()`; that is oven-sh#33461's subject. - `fetch()` upload framing: a blob/bytes-backed or closed-empty stream body goes out with a Content-Length (as 1.4.3 did for the blob case through the eager transfer, and as undici does for bodies whose source it knows). A file-backed stream keeps streaming chunked, as today, because its length may not be knowable (FIFO, device). A JS stream streams chunked. - `Response.redirect()`, `Response.error()` and the S3 `new Response(s3file)` redirect used `Value::Empty`. The spec body is null. With `Empty` now tracked like any other body they would have become visibly one-shot, so they are `Value::Null` here (the same three-line change sits in oven-sh#33125). - `Bun.readableStreamToText()` and the other helpers on a stream a Body already consumed reject with `ERR_INVALID_STATE` "ReadableStream has already been used" (a stream held by someone else's reader still says "is locked"). `test/js/web/streams/readable-stream-blob-consumed.test.ts` asserted `ERR_BODY_ALREADY_USED` from the old blob-loader path and is updated; its point (no crash, a rejected promise) is unchanged. - Rebased onto oven-sh#42053: its `take_blob_from_unread_stream` used `force_detach`; `to_any_blob` now marks the stream consumed itself. - Suites run on the debug build: `body.test.ts`, `body-stream.test.ts` (9086), `body-clone`, `body-mixin-errors`, `body-async-iterator`, `body-stream-excess`, `serve.test.ts`, `bun-server`, `bun-serve-static`, `bun-serve-file`, `bun-serve-body-json-async`, `serve-if-none-match`, `proxy.test.ts`, `cookie.test.ts`, `html-rewriter.test.js`, `bun-write.test.js`, `spawn-stdin-readable-stream`, `streams.test.js`, `readable-stream-blob-consumed`, `native-source-onclose-leak`, `sync-pull-fast-path`, `request.test.ts`, `response.test.ts`, `client-fetch`, `content-length`, `fetch.stream`, `fetch.test.ts`, `fetch-abort-stream-body`, `fetch-keepalive`, `fetch-backpressure`, `node-stream.test.js`, `direct-readable-stream`, the node `test-readable-from-web-*` files, regression 07001 and 09555. The failures left in `fetch.test.ts`/`serve.test.ts`/`bun-server`/`fetch-backpressure` are environment-only here (IPv6, running as root, no internet or S3 egress, ASAN timeouts, and the ASAN RSS bound in "bounds memory when a handler forwards req.body") and reproduce with `origin/main`'s `src/`. </details>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
process.exit()callsprocess.reallyExit(code), as Node.js does. Whenprocess.reallyExitis not callable, Bun throws aTypeErrorwith an empty message. Node.js throwsTypeError: process.reallyExit is not a function.ASSERTION FAILED: !message.isEmpty()atvendor/WebKit/Source/JavaScriptCore/runtime/Error.cpp(71)inJSC::createTypeError.JSC::call(globalObject, reallyExitVal, args, ""_s)atsrc/jsc/bindings/BunProcess.cpp:913. The last argument is the message for a value that is not callable. This overload also passes the function as its ownthis. Node.js passesprocess.Fix
processasthisand the messageprocess.reallyExit is not a function.process.reallyExit(process.exitCode || 0)in JavaScript. That is a method call onprocess, and V8 gives that message.test/js/node/process/process.test.js. Both fail on release 1.4.3-canary.1. Also ran the otherprocess.exit()tests and 12 exit tests fromtest/js/node/test/parallel/.Background
process.reallyExitis the raw exit of Node.js: it emits no'exit'event. Libraries such as signal-exit replace it.JSC::callis the JavaScriptCore helper that calls a JS function from C++.JSC::callinsrc/has an empty message.Downsides
process.reallyExitnow seesthis === processwhenprocess.exit()calls it. Before,thiswas the function itself.BunProcess.cppobject (-O3, no LTO): 37 for the message, 16 of code. Per call: no new allocation, branch, or syscall.Notes
Repro:
TypeError:ASSERTION FAILED: !message.isEmpty()TypeError: process.reallyExit is not a functionTypeError: process.reallyExit is not a functionThe same result for each value tried:
"str",undefined,null,1,{},Symbol("x"), anddelete process.reallyExit.The
thisvalue, withprocess.reallyExit = function () { console.log(this === process) }andprocess.exit(3):falsebefore,trueon this branch and on Node.js v26.3.0.Not changed by this PR: when nothing catches the
TypeError, Node.js exits with the code given toprocess.exit()(0 above, 5 forprocess.exit(5)). Bun exits 1. A replacedprocess.reallyExitthat throws shows the same difference. For an entry point that throws,uncaught_exception(src/jsc/VirtualMachine.rs:2161) andexit_with_unhandled_note(src/runtime/cli/run_command.rs:1638) both set exit code 1. Related to #42032, which changes the first place for an'exit'listener that throws. The new test catches the error in the child, so it does not depend on that exit code.History: #16026 added the call with the comment
// process.reallyExit(exitCode);.Search for other call sites:
JSC::call,call,profiledCallandconstructwith an empty message literal insrc/andpackages/.BunProcess.cpp:913is the only one.Size measurement: compiled
BunProcess.cppfromorigin/mainand from this branch with the release flags of the build (-O3 -march=nehalem), without-flto=thinso that the object is native code.size -A:.textofBun::Process_functionExit712 to 728 bytes,.rodata.str1.110594 to 10631 bytes,.dataand.bssunchanged.On a debug build without the fix, the first new test fails: the child exits with code 134 and the assertion is in its stderr.
Tests run on the debug build of this branch:
test/js/node/process/process.test.js -t "process.exit\(\)": 6 pass.test/js/node/process/process.test.js, whole file: 179 pass, 5 skip, 3 fail.processfails because the container sets noUSER.signal > simple case worksandsignal > process.emit will call signal eventstime out at 5 s in the whole-file run, with and without the fix. Each passes in 2.5 s when it runs alone.test/js/node/test/parallel/:test-process-exit-code-validation.js,test-process-exit-code.js,test-process-exit-from-before-exit.js,test-process-exit-handler.js,test-process-exit-recursive.js,test-process-exit.js,test-process-really-exit.js,test-worker-exit-code.js,test-worker-nested-on-process-exit.js,test-worker-on-process-exit.js,test-worker-process-exit-async-module.js,test-crypto-op-during-process-exit.js: all exit 0.Not run: macOS, Windows. The changed line has no platform-specific code.
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