Repository navigation
Hardening: fix crashes across the runtime, bundler, dev server, shell and CLI - #46021
Conversation
|
Updated 4:05 PM PT - Oct 20th, 2026
@Jarred-Sumner, your commit f48d297 is building: |
…nd two CI test failures
… an ordinary buffer is not moved again (#42192)
…ueue and Windows watch no others
…ver test on Windows
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline findings, I also checked the Windows us_loop_free rework in packages/bun-usockets/src/eventing/libuv.c (uv_walk + bounded uv_loop_close retry) — every loop Bun creates passes a null hint to us_create_loop, so usockets owns the uv_loop it walks, the wait is deadline-bounded, and a loop that never closes is leaked rather than freed, so it is not a crash or misuse of a foreign loop.
Extended reasoning...
The us_loop_free change only runs on loops where is_default is 0, which is every loop in src/ since all us_create_loop call sites pass a null hint; the retry loop breaks at a 10s deadline and skips freeing the uv_loop when uv_loop_close still reports busy.
Findings marked 🟡 are optional suggestions and need no follow-up push.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🔴
src/jsc/bindings/sqlite/JSSQLStatement.cpp— A bun:sqlite user whose connection has a commit/update hook or user function can hit a use-after-free of a finalized statement, which the base never did. jsSQLStatementFunctionIterateEnd copies castedThis->stmt into a local at JSSQLStatement.cpp:2272 and only then runs CHECK_NOT_BUSY at :2275. That macro calls finalizeOrphans (:2710), which sqlite3_finalize()s GC-orphaned statements and so runs user hook JS. If that JS calls stmt.finalize() or db.close(true), the local stmt is freed, and :2278 passes it to sqlite3_reset. Fix: re-read castedThis->stmt after any CHECK_NOT_BUSY / finalizeOrphans and bail when it is null, at every site that captures stmt or db before the macro.Why this was flagged
Trigger: a connection with a hook, user function or authorizer (Bun::sqliteCommitsWhenFinalized) has a GC-orphaned statement in VersionSqlite3::orphans (JSSQLStatement.cpp:193); user code then calls the iterator's return()/end, reaching jsSQLStatementFunctionIterateEnd. The local
stmtis read at :2272 before CHECK_NOT_BUSY at :2275. CHECK_NOT_BUSY (:160-161) calls finalizeOrphans (:2710), whose sqlite3_finalize commits and runs the user's commit hook JS, and reportUncaughtExceptionAtEventLoop (:2718) can also run JS. If that JS calls stmt.finalize() on the same statement or db.close(true), castedThis->stmt is nulled and the sqlite3_stmt freed, but the stale local is still used at :2278 (sqlite3_reset(stmt), sqlite3_db_handle(stmt)): heap use-after-free. On the base branch orphans did not exist; finalization happened in the GC finalizer so no user JS ran between the null check and the use. Remedy: order CHECK_NOT_BUSY before any read of castedThis->stmt and re-check stmt for null afterwards.Verification: In src/jsc/bindings/sqlite/JSSQLStatement.cpp,
jsSQLStatementFunctionIterateEndcopiesauto* stmt = castedThis->stmt;at :2272 and only then runs CHECK_NOT_BUSY at :2275. That macro (:160-161) now callsfinalizeOrphans. :2278 callsrunCount(stmt)andsqlite3_reset(stmt)on the stale local. In the base file CHECK_NOT_BUSY (base :155-159) is a pureinCallbackcheck. -
🟣
src/runtime/node/node_fs.rs:2881— pre-existing: async fs.readv/fs.writev on a view of a non-shared WebAssembly.Memory still reads or writes freed memory after grow(), while this PR fixes only fs.read. FdVectorIo::from_js at node_fs.rs:2873-2881 pins the backing stores, but a pin does not keep a Wasm block alive (array_buffer.rs:757 comment), so the iovecs captured there point at a freed block once memory.grow() runs before the pool thread's readv/preadv (node_fs.rs:6276) or uv_fs_read (node_fs.rs:869). Fix: give the vector path the same protection fs.read got at node_fs.rs:4159-4163, so every async fd I/O argument (read, readv, write, writev) copies or uses scratch for buffers that can_be_freed_while_pinned and writes back after completion.Why this was flagged
A script calls fs.readv(fd, [new Uint8Array(mem.buffer)], cb) where mem is a non-shared WebAssembly.Memory, then calls mem.grow(1) before the callback runs. FdVectorIo::from_js (node_fs.rs:2873-2881) collects iovec pointers via Bun__JSArray__collectBufferSpans (bindings.cpp:7065-7071), which only pin()s the ArrayBuffer. The diff's own doc on ArrayBuffer::can_be_freed_while_pinned (array_buffer.rs:757-760) states grow() detaches a non-shared Wasm memory's buffer regardless of the pin count and frees the old block. The pool thread then runs Syscall::preadv/readv (node_fs.rs:6276, 6288) into that freed block: either EFAULT surfaces as a spurious I/O error, or, if the allocator reused the pages, the kernel writes file bytes over live memory. This PR added write_to_scratch_if_wasm/write_back for args::Read only (node_fs.rs:4159-4163, 1131-1134); the base branch had the same exposure, so readv/writev remain as on base. The pin at node_fs.rs:2878-2881 looks like the safeguard but, per array_buffer.rs:757, does not cover Wasm memory.
Verification: node_fs.rs:2873-2881 FdVectorIo::from_js calls VectorArrayBuffer::from_js; nothing copies or scratches Wasm-backed views. array_buffer.rs:184-187: grow() detaches a non-shared WebAssembly.Memory's buffer regardless of the pin count. The raw iovecs are consumed off the JS thread at node_fs.rs:6276/6288. No equivalent of the fs.read fix (node_fs.rs:4159-4163) exists on the vector path.
There was a problem hiding this comment.
Still open from earlier reviews (5):
- 🔴
src/runtime/napi/uv_posix.rs:370—A native addon thread that keeps calling uv_async_send faster than its callback returns can pin the JS thread inside on… - Also unresolved: 4 minor or pre-existing.
If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.
…e converting arguments, and smaller fixes
… close a dev server file descriptor leak
…h-dir-event-lazy-abs-path main took the read-error guard and the path bounds checks of this branch in #46021. Conflicts: - src/jsc/hot_reloader.rs: the directory arm stays as on this branch and reads only the watchlist. `cached_dir_entry` from main has no caller left and goes with the tombstones. The new function uses `FileSystem::top_level_dir()` and `WatchItemIndex`. - src/watcher/Watcher.rs: `WatchItemIndex` is `u32` as on main, the eviction list stays a `Vec`. - src/watcher/lib.rs: both sets of exports. - test/cli/hot/hot.test.ts, test/cli/watch/watch.test.ts: the tests of main, then the tests of this branch.
…orted form main has the two long-path tests since #46021, so the copies of this branch go. The abort on a read-error listing is fixed on main too, so the two `.env` rows, which guarded a second way to that abort, repeat the `c.ts` rows and go as well. The rows that stay are named for what they now prove: a later save still reloads. Without the abort a lost reload is a hang, so their time limit is finite on a debug build. New test: `--hot` with an import that is saved by rename after a failed `import()` in the same folder.
…on::entries` to the resolver `WatchList::ensure_unused_capacity` reserves the table too, so `append_assume_capacity` does not allocate. `EntriesOption::entries` has two callers in the resolver and none in the reloader since #46021, so this branch no longer deletes it.
Remove the debug-only timeout of the two tests. The default applies. Each child keeps two threads busy: the compress loop and the writer. On a debug build, two children at once take 2.3 to 3.4 s each (median of two batches), which is close to the 5 s default. One child at a time takes 1.9 s. On a release build from before #46021 the two tests fail as often either way: 104 of 120 tests with two children at once, 102 of 120 with one at a time.
- `JSValue::get_own_non_observable` borrows the name through `StringView`. `String::borrow_utf8` is gone since #46825. - The two JSX props iterators set `include_symbols` (added in #41910) to `false`, which is what the observable iterator does on main. - `JSC__JSValue__getOwnNonObservable` builds its property key with `Bun::toIdentifier`, like `JSC__JSValue__getOwn` since #46021. - The two new `Bun.inspect` tests move to the JSX tests at the end of `inspect.test.js`. Above the `ErrorEvent` test they pushed its source line past 999, and the wider line-number gutter broke its inline snapshot.
What does this PR do?
Hardening. This collects the open pull requests that fix a crash in Bun into one change, and fixes about 20 more crashes that had no pull request.
"Crash" here means a segfault, a Rust panic, an abort, a failed assertion, memory corruption that a sanitizer reports, a native stack overflow, or an exception that ends the process and that the program can't catch. With this PR those cases throw a normal error, print a normal CLI error and exit 1, or just work.
306 pull requests are included: 227 whole, and 79 cut down to the crash fix where the rest was a refactor, a feature or a behavior change. Each one was checked against current
mainfirst. 45 more turned out to be fixed already, and 17 were duplicates. The full list is at the bottom.Crashes you could hit in normal use
bun buildwith macros and a.envfile crashed at random. In a repro with many macro modules, more than a third of the builds crashed. Now 3,000 of 3,000 produce the right bundle. (macros: give each parse-worker macro VM its own env loader #45960)bun runandbunxaborted if a signal arrived while the child was being spawned. (Clear the inherited signal mask at startup and exec children with an empty one #38926, bun run: restore the signal mask after it forwards a pending signal to the script #45965)bun build --compile ./index.js ./public/*panicked if an embedded file's name wasn't valid UTF-8.Option::unwrap()panics after a failed or deleted import (bun build --server-components: require the bun target and reject directives without a framework #38046, dev server: answer a cached-file lookup only for a graph that bundled the file #44744, dev server: keep an importer in its own graph when an import fails or is deleted #44746), a segfault when anonLoadplugin throws while another bundle is queued, a request that hung and then panicked withbundle active, andserver.stop()that never resolved after a client aborted a waiting request.Bun.serve:server.fetch(url, { headers: new Headers() })followed by a GC segfaulted (server.fetch: copy a Headers argument instead of adopting the wrapper's pointer #40888). Routes were used after free afterserver.reload()withtls.serverName(serve: fix route use-after-free after reload() with tls.serverName #38636).text()andjson()on a stream of string chunks could abort the process, a regression in 1.4.0. (streams: join the string chunks of a text consumer with one allocation #44269)Bun.Transpilerwithexports.eliminateorexports.replaceaborted onexport default 1and on most re-export clauses. (js_parser: fix four crashes of exports.eliminate and exports.replace #44137)declarefield in a class expression withexperimentalDecoratorspanicked. (js_parser: fix panic on decorated declare/abstract fields in class expressions with experimentalDecorators #38095)--no-bundleand inBun.Transpiler. (bundler: handle YAML and JSON5 keys that are not strings instead of panicking #41827)Bun.FileSystemRouter: reading.paramssegfaulted when a rejected route candidate left its parameters behind. (FileSystemRouter: match / against a root-level optional catch-all route #39290, FileSystemRouter: order dynamic routes by per-segment precedence like Next.js #39291)uv_async_*,uv_timer_*,uv_idle_*,uv_prepare_*,uv_check_*,uv_queue_workoruv_cwdaborted with "unsupported uv function" on Linux and macOS. They're implemented now. (napi: implement uv_cwd on POSIX #36609, napi: implement uv_async_t, uv_queue_work and uv_default_loop on posix #39652, napi: implement uv_idle_t, uv_prepare_t, uv_check_t and uv_timer_t on posix #39892)--watchand--hotpanicked when the system was out of inotify instances or file descriptors, or when the reloadexecfailed. They print an error now. (watch: report a failed watcher init as an error instead of a crash #39629, watch: report a failed reload exec as an error instead of a panic #39975, --hot: defer a reload during the preloads, and report a reload that cannot start #44837)bun install, the resolver, the bundler,--compile,tsconfig.jsonpathsandbaseUrl,package.jsonmainandsideEffects,bun run --filter,bun pm diff, source maps and snapshots. They're ordinary errors now, or they work. (paths: size the thread-local join output buffers to MAX_PATH_BYTES on Windows #36340, resolver: don't abort on a package.json sideEffects entry longer than the join buffer #37529, bun test: report over-long coverage dir, JUnit outfile and bunfig root instead of panicking #39104, bun run --filter: stop panicking on workspace paths longer than the path buffer #39650, Report a path that does not fit a path buffer instead of aborting #43067)ShadowRealmand importsnode:worker_threadsin it segfaulted.structuredCloneorpostMessageof an X25519 key gave an object that aborted the process when read. (webcrypto: write the key type into the X25519 structured-clone record #45624)node:streamweb adapters andnode:http2could end the process with an exception that no handler could catch. (node:http2: let close() and destroy() win over the refusal of a request #45076, node:stream: do not touch the web controller after the web side is done #45975)node.exewithout the delay-load hook (napi(windows): load addons that import node.exe without win_delay_load_hook #35618).Most of the rest needs unusual input: strings near the 2 GiB limit, buffers that another thread or a getter resizes or detaches in the middle of a call,
worker.terminate()at a bad moment, running out of memory or file descriptors, very deep nesting, or hostile arguments tobun:ffi,node:vmandbun:sqlite.Behavior changes
Things that didn't crash before and behave differently now:
newon native functions that aren't constructors throws aTypeError, as in Node:util.types.*,buffer.isAscii,isUtf8,transcode,resolveObjectURL,bun:jscexports,cc()symbols. It used to return the call's result. They also stop printing as[class hostname]. (Don't register native functions as their own constructors #34575)$`cmd`throwsERR_INVALID_ARG_VALUEif an environment variable's name or value contains a NUL byte. It used to cut the value at the NUL. (shell: reject NUL bytes in .cwd() and .env() at the native setters #44981)cc()functions throw aTypeErrorfor arguments that used to reach C as garbage, andread.*(ptr, NaN)throws. (bun:ffi: stop cc() from turning non-numeric pointer arguments into garbage pointers #37989, bun:ffi: make cc() reject non-numeric i64/u64 arguments instead of passing garbage #38057, bun:ffi: add the full byteOffset in read.*, do not truncate it to int32 #40773)bunexits with 128 + the signal number on Ctrl+C ordocker stop. (cli: exit 128 + signo when bun cannot die from a re-raised signal (bun as container PID 1) #38877)NODE_MODULEaddon that's already loaded by the main thread throwsERR_DLOPEN_FAILEDwhen a live Worker loads it too, as in Node. (process.dlopen: replay a non-context-aware V8 addon only while no other live VM owns it #39968)node:vmcontext reachesuncaughtExceptionhandlers. (process: deliver exceptions reported against a node:vm context to uncaughtException #39992)vm.constants.DONT_CONTEXTIFYreturns the real global proxy. (node:vm: run DONT_CONTEXTIFY contexts directly against the real global #34623)createPrivateKeyrejects an EC JWK whose private value doesn't match its public point. (node:crypto: validate EC JWK private scalar matches public point in createPrivateKey #33752)@dec static {}is a syntax error, as in TypeScript and esbuild. (js_parser: reject a decorator on a class static block, keep decorators out of the body ofdeclare global#44883)clearTimeoutignores 0 and negative ids, and timer ids no longer wrap. (timers: allocate ids from one 64-bit counter that never wraps #44771)Responsewhose body is an S3-backedBlobreports the length that GET sends. (Bun.serve: answer HEAD for an S3-backed Blob body without an S3 stat #44734)bun:sqliteandnode:sqlite: ifiterate()left a write unfinished on a connection that has a hook, a user function or an authorizer, the write is finished on the connection's next call instead of during garbage collection. If the database is gone too, it's rolled back.bun test --bailruns exit listeners and closes SQLite databases. (bun test: exit through the VM teardown path when --bail stops a run #39010)File formats
.jscbytecode files get a 24-byte footer so they can be verified before decoding. Files from an older Bun are ignored and the source is parsed instead. (bundler: frame .jsc bytecode sidecars and verify them before decoding #39396)node:vmcachedDatagets an 8-byte checksum. A blob from an older Bun is rejected, which the API allows. (node:vm: reject invalid cachedData instead of crashing #32839)Performance
Measured in instructions per operation on release builds of this branch and its base, with an A/A run for the noise floor (0.0 to 0.2 %).
No change:
Bun.serve,node:http, WebSocket echo and publish,bun buildof 3,807 modules,bun runwith 2,000 imports,readdir/Glob/fs.cpover 23,689 entries, startup and RSS. The binary grows by 176 KB (0.17 %).Bun.inspectof a large non-ASCII stringBun.markdown.htmlon large non-ASCII input$`cmd`with 300 environment variablesfs.readSyncTextEncoder.encodeInto, short ASCII stringscrypto.pbkdf2, 16-byte inputscc()callsclose()(#37989, #38057, #43802, #44921).dlopen()calls don't changeprocess.env.X = vcrypto.pbkdf2, inputs of 1 KB or morebun buildwith an object--defineused thousands of timesNot included
new CString(ptr)still returns a string #44226. Pin the storage of a typed array that has no ArrayBuffer while native code borrows it #42185 too: a typed array that has noArrayBufferyet can't be pinned, soview.buffer.transfer()during an async call can still free its bytes. Creating theArrayBufferup front costs 7 to 12 % on every asyncfscall, so the fix belongs in JSC: anArrayBuffercreated while the view is borrowed should start out pinned.calc()sums as a flat list): it changes minified output for real stylesheets to survive acalc()with 25,000 terms. It deserves its own review.How did you verify your code works?
main(a release or an ASAN build) and passes on this branch, except where that isn't practical: code that only runs on Windows or macOS, inputs that need several GB of memory, and a few rare races. Those were checked by hand or by reading.main, side by side. Release: 88 no longer crash. ASAN: 106 no longer crash. None crashes that didn't before. What still crashes needs WebKit, is SQLite's own bug, or isn't a crash. The ASAN number is from a later commit than the release number.mainhung in 82 of 1,161 programs. This also found the four dev server bugs listed above.main. No sanitizer report, and the results match wherevermaindoesn't panic.server.reload()change, and a changed error message forimport("file:///50%deposit.ts").Included pull requests
Bundler, transpiler, CSS, macros (33)
graph.astappend #39855 bundler: join the work still on the pools before the bundle is torn down (crash fix only)varfor a data file with no named export #43330 transpiler: do not print an emptyvarfor a data file with no named exportdeclare global#44883 js_parser: reject a decorator on a class static block, keep decorators out of the body ofdeclare global(crash fix only)bun build --compileand standalone executables (3)Resolver, module loading, plugins, paths (25)
thisin Module.prototype.require #42401 node:module: accept anythisin Module.prototype.require (crash fix only)__esModulegetter during the annotation lookup #45117 cjs: do not call an__esModulegetter during the annotation lookupDev server,
bun bake, routers,--watchand--hot(28)Bun.serve, sockets, HTTP, WebSocket, HTTP/2, QUIC, DNS (28)Streams, Blob, S3, files,
node:fs(28)Node-API, V8 API,
bun:ffi,process.dlopen(15)node:vm, workers, process, timers (19)Shell, spawn, CLI,
bun test,bun install(31)ackandaddressTypein the primary only when they are int32 #43838 node:cluster: read a worker'sackandaddressTypein the primary only when they are int32Web APIs, Buffer, encoding, URL, crypto, zlib, SQLite, Redis and the rest (96)
> ${buf}redirect to the target buffer's live length #42179 shell: clamp a> ${buf}redirect to the target buffer's live lengththiswith putDirect: process.ppid, require.cache, StringDecoder, the native EventEmitter #42575 Stop natives from writing onto a foreignthiswith putDirect: process.ppid, require.cache, StringDecoder, the nat… (crash fix only)file:URL whose path is not UTF-8 on Linux and macOS, through onefileSystemPath()accessor #45354 Throw for afile:URL whose path is not UTF-8 on Linux and macOS, through onefileSystemPath()accessor (crash fix only)?or a class matches when an alternative takes 64 bytes or more #45653 glob: a group with?or a class matches when an alternative takes 64 bytes or morebun check: fix a panic onexport *without a module specifier, and a false error for one in parentheses #45832bun check: fix a panic onexport *without a module specifier, and a false error for one in parenthesesAlready fixed on
main, or duplicates of a pull request aboveAlready fixed: #27323 #30401 #31011 #31206 #31566 #31626 #31695 #31702 #31945 #32773 #34154 #34697 #34744 #34964 #35655 #35816 #36020 #36181 #36296 #36342 #36581 #36602 #36807 #36818 #36857 #37172 #37235 #37239 #37267 #37420 #37731 #37914 #38113 #38119 #38509 #38514 #38540 #38975 #39410 #40749 #41163 #41171 #42108 #43085 #43230
Duplicates: #45654 #29986 #33378 #36895 #37070 #37639 #38014 #39021 #41266 #42250 #42951 #43238 #44259 #45335 #45565 #45726 #45965
Fixes #5019
Fixes #6106
Fixes #10690
Fixes #10886
Fixes #13566
Fixes #14745
Fixes #15551
Fixes #20454
Fixes #21310
Fixes #29145
Fixes #30454
Fixes #30613
Fixes #30766
Fixes #32103
Fixes #32142
Fixes #32183
Fixes #32212
Fixes #39060
Fixes #42296
Fixes #42931
Fixes #43671
Fixes #44201
Fixes #44735
Fixes #44783
Fixes #45198
Fixes #45327
Fixes #45491
Fixes #45721