deps: update sqlite to 3.53.100 - #2
Merged
Merged
Conversation
github-actions
Bot
force-pushed
the
deps/update-sqlite
branch
from
April 12, 2026 07:19
ed7383d to
66ff949
Compare
springmin
pushed a commit
that referenced
this pull request
May 5, 2026
…ven-sh#29971) ## What `FileSystemRouter`'s constructor (and `reload()`) initialize the error log with the arena allocator: ```zig const allocator = arena.allocator(); ... var log = Log.Log.init(allocator); ``` When route loading produces errors, the error paths did: ```zig arena.deinit(); globalThis.allocator().destroy(arena); return globalThis.throwValue(try log.toJS(...)); // reads arena-backed msgs.items ``` `log.msgs.items` is backed by the arena, so `log.toJS()` reads freed memory. ASAN reports `use-after-poison` in `logger.Log.toJS`. ## Repro ```js // pages/[foo.tsx — missing closing bracket new Bun.FileSystemRouter({ style: "nextjs", dir: "./pages", fileExtensions: [".tsx"] }); ``` Debug (ASAN) build: ``` AddressSanitizer: use-after-poison ... #1 in logger.Log.toJS (src/logger.zig:733) #2 in FileSystemRouter.constructor (src/bun.js/api/filesystem_router.zig:149) ``` ## Fix Build the JS error value first (while the arena is still live — `BuildMessage.create` / `ResolveMessage.create` clone the msg into `globalThis.allocator()`), then free the arena, then throw. Applied to all four `log.toJS()` call sites across `constructor()` and `reload()`. ## Verification - `git stash -- src/ && bun bd test filesystem_router.test.ts -t 'invalid route'` → **fail** (ASAN crash in subprocess) - `git stash pop && bun bd test filesystem_router.test.ts -t 'invalid route'` → **pass**, error message is `Route is missing a closing bracket]` - All 19 existing `filesystem_router.test.ts` tests pass. --------- Co-authored-by: robobun <robobun@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
May 5, 2026
…es (oven-sh#30077) ## What When a chunked (or HTTP/3) request body exceeds `maxRequestBodySize`, `onBufferedBodyChunk` writes the 413 directly on the raw uWS response: ```zig resp.writeStatus("413 Payload Too Large"); resp.endWithoutBody(comptime !http3); ``` `internalEnd` → `markDone()` nulls `onAborted`, so when the socket closes no abort ever fires to detach `ctx.resp` or release the base ref. `this.resp` is left pointing at a completed response whose socket is about to be freed by `us_internal_free_closed_sockets`. If the fetch handler returned a pending Promise: - **resolve**: `handleResolve` → `isAbortedOrEnded()` is false (`this.resp != null`) → `render()` → `runCorkedWithType` corks the freed socket → **heap-use-after-free** (ASAN trace below). - **reject**: `handleReject` reads `resp.hasResponded()` off freed memory, sees `true`, skips the error handler, and returns without ever releasing the base ref → **RequestContext leaks** (`server.pendingRequests` never returns to 0). ## Fix Route through `this.endWithoutBody()` (the `RequestContext` wrapper) instead of the raw `resp.endWithoutBody()`. That path does `detachResponse()` (nulls `this.resp`, clears `onData`/`onAborted`/`onTimeout`) and `deref()` (releases the base ref), matching every other end path in this file. The body promise is rejected with the specific `"Request body exceeded maxRequestBodySize"` error *before* `endWithoutBody()` so `endRequestStreaming()` doesn't overwrite it with a generic `ConnectionClosed`. `has_written_status` is set so any later `renderMissing`/`renderMetadata` knows the status line is already committed. ## Repro ``` ==ERROR: AddressSanitizer: heap-use-after-free #0 us_socket_group socket.c:77 #1 uWS::AsyncSocket<false>::getLoopData() AsyncSocket.h:69 #2 uWS::AsyncSocket<false>::isCorked() AsyncSocket.h:141 #3 uWS::HttpResponse<false>::cork(...) HttpResponse.h:647 #4 uws_res_cork libuwsockets.cpp:1740 #5 ...runCorkedWithType Response.zig:299 #6 ...doRenderBlob RequestContext.zig:1942 ... #11 ...handleResolve RequestContext.zig:220 #12 ...onResolve RequestContext.zig:154 freed by: #1 us_poll_free epoll_kqueue.c:73 #2 us_internal_free_closed_sockets loop.c:305 ``` ## Test `test/js/bun/http/serve-pending-promise-abort-leak.test.ts` — new case sends a raw `Transfer-Encoding: chunked` POST exceeding `maxRequestBodySize` with a handler that holds its resolve/reject, waits for the socket to be reclaimed, then settles the Promise. Asserts `pendingRequests` returns to 0 for both paths, the body was rejected with the right message, and a follow-up request still works. Without the fix: ASAN heap-use-after-free on the resolve path; on release builds the reject path shows `pendingAfterReject: 1` (leak). Co-authored-by: robobun <robobun@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
May 5, 2026
…to prevent UAF (oven-sh#30057) ## What `UDPSocket.sendMany()` and `UDPSocket.send()` both captured raw pointers into the payload's ArrayBuffer backing store (or borrowed `WTFStringImpl` storage for Latin-1 strings) and then hit JSC safepoints before handing those pointers to `bsd_sendmmsg`: - **`sendMany`**: subsequent loop iterations call `iter.next()` (slow path → `JSObject.getIndex`), `coerceToInt32` on the port, and `toBunString` on the address - **`send`**: `parseAddr` calls `coerceToInt32` on the port and `toBunString` on the address after the payload is captured Any of these can run user JS that detaches an earlier payload's ArrayBuffer via `.transfer(newLen)` (which synchronously frees the old backing store) or drops the last reference to a JSString, leaving the captured pointer dangling. ## Repro ```js const buf = new ArrayBuffer(4096); const payload = new Uint8Array(buf); const evilPort = { valueOf() { buf.transfer(0); // synchronously frees the 4096-byte backing store return server.port; }, }; client.sendMany([payload, evilPort, "127.0.0.1"]); // or client.send(payload, evilPort, "127.0.0.1") // bsd_sendmmsg reads 4096 bytes from the freed region ``` Under ASAN (with `Malloc=1` so bmalloc routes through the system heap): ``` ==…==ERROR: AddressSanitizer: heap-use-after-free on address … at pc … READ of size 4096 at … thread T0 #0 … in read_iovec(…) #2 … in sendmmsg #3 … in bsd_sendmmsg packages/bun-usockets/src/bsd.c:123 freed by thread T0 here: … #14 … in JSC::arrayBufferCopyAndDetach(…) JSArrayBufferPrototype.cpp:365 … oven-sh#30 … in JSC::JSValue::toInt32(…) ← parseAddr's coerceToInt32 ``` ## Fix - **`sendMany`**: root every payload JSValue in a `MarkedArgumentBuffer` for the duration of the call and split the loop into two phases. Phase 1 collects/validates payload JSValues and runs all user-JS re-entrance (`iter.next`, `parseAddr`). Phase 2 borrows byte slices from the rooted JSValues once no more user JS sits between capture and `socket.send`. GC cannot collect a rooted payload; an ArrayBuffer that was detached during phase 1 reports a zero-length slice instead of a dangling pointer. No payload bytes are copied. - **`send`**: reorder so `parseAddr` runs before the payload pointer is captured. `payload_arg` stays rooted in the callframe, and nothing between capture and `socket.send` hits a JSC safepoint — so no copy is needed. ## Verification - **Without fix:** `bun bd test test/js/bun/udp/udp_socket.test.ts -t 'detaching an ArrayBuffer'` → ASAN heap-use-after-free in `read_iovec` → `bsd_sendmmsg` for both `send` and `sendMany`, tests fail - **With fix:** both tests pass; received bytes match the original payload - Full `test/js/bun/udp/` suite (207 tests) passes - `zig:check-all` passes on all targets --------- Co-authored-by: robobun <robobun@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
May 5, 2026
## Problem
`MarkedArrayBuffer.destroy()` did two things:
```zig
allocator.free(content.buffer.slice()); // free the bytes
allocator.destroy(this); // free *this
```
Every constructor that is actually used (`fromString`, `fromBytes`,
`fromJS`, `fromTypedArray`, `fromArrayBuffer`) returns
`MarkedArrayBuffer` **by value**, so `this` is never an individually
heap-allocated struct — it's a stack local, an embedded field, or an
ArrayList slot. The `allocator.destroy(this)` call passes that interior
pointer to mimalloc.
In the readdir Buffer error-cleanup path (`readdirWithEntries` /
`readdirInner`), entries are appended by value via
`Buffer.fromString()`:
- `allocator.destroy(&entries.items[0])` frees `entries.items.ptr`
- the next loop iteration reads `this.*` from poisoned memory
- `entries.deinit()` frees the same pointer again
## Repro
```js
const fs = require('fs');
// dir contains regular files + a self-referential symlink 'loop -> loop'
fs.readdirSync(dir, { encoding: 'buffer', recursive: true });
```
The recursive walk collects Buffer entries for the root, then fails with
`ELOOP` opening the symlink (not in the swallowed `NOENT/NOTDIR/PERM`
set), and enters the cleanup loop. Under ASAN:
```
==3593==ERROR: AddressSanitizer: use-after-poison on address 0x737ec6e50040
READ of size 64 at 0x737ec6e50040 thread T0
#1 MarkedArrayBuffer.destroy array_buffer.zig:591
#2 NodeFS.readdirInner node_fs.zig:5013
#3 NodeFS.readdir node_fs.zig:4518
```
## Fix
- Drop `allocator.destroy(this)` from `MarkedArrayBuffer.destroy()`. The
struct is passed/stored by value; callers own its storage.
- Remove the unused `MarkedArrayBuffer.init()` (the only function that
heap-allocated the struct, zero callers) so there's no pairing that
would leak.
- The readdir call sites keep calling `.destroy()`, which still checks
`this.allocator` before freeing bytes — JS-owned buffers remain
untouched.
Also fixed the adjacent `Dirent` arm of the recursive-sync error
cleanup: `result.name.deref()` → `result.deref()` so `Dirent.path` is
released too (matching the non-recursive and async cleanup sites).
## Verification
New test in `test/js/node/fs/fs.test.ts` creates a temp dir with files +
a self-referential symlink, spawns a subprocess that calls
`readdirSync({encoding:'buffer', recursive:true})`, and asserts it
throws `ELOOP` and exits 0.
```
# without fix
(fail) readdirSync({encoding: 'buffer', recursive: true}) frees entries safely ...
{ exitCode: 134, stdout: "" } # SIGABRT from ASAN
# with fix
(pass) readdirSync({encoding: 'buffer', recursive: true}) frees entries safely ... [1.5s]
{ exitCode: 0, stdout: "ELOOP" }
```
`zig:check-all` passes on all targets.
---------
Co-authored-by: robobun <robobun@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
May 5, 2026
…ss (oven-sh#30181) ## What Surveyed ~40 recent CI builds for bake test flakes and fixed the underlying causes. ### Flake #1 (93 hits): `dev-and-prod-12: hmr handles rapid consecutive edits` Two modes: - **Windows**: `Bun.write` is `open(O_TRUNC)` then async write with a JS-thread round-trip in between. The watcher fires on the 0-byte truncation, bundles an empty module that never calls `accept()`, and the next update falls through to `fullReload()` → client exits `unexpectedReload`. - **All platforms**: after the final drain `client.messages.length = 0`, a late hot_update lands during the following `await client.js\`…\`` and trips the unread-messages disposal check. **Fix (test):** use `fs.writeFileSync` for the rapid burst (microsecond truncate window), write identical content so same-`sourceMapId` duplicates are deterministic on every platform, and follow with a synchronized sentinel write — once the sentinel arrives over the ordered WS, every prior hot_update has been applied and nothing can leak into disposal. ### Flake #2 (29 hits): `Timeout waiting for line "… socket connected"` Across `react-spa`, `html`, `ssg-pages-router`, `bundle`, `hot`, `esm`, `css`, `incremental-graph-edge-deletion`. `waitForLine()`'s default timeout was **1 second** on non-Windows release builds — the Node client has to start, import happy-dom, fetch, parse HTML, run the bundle, and open a WebSocket in that window. The `ASAN_TIMEOUT_MULTIPLIER` constant existed but was never applied. **Fix (harness):** raise the base and apply a unified `WAIT_MULTIPLIER` (debug × ASAN × CI). Apply the same multiplier to `expectMessage` / `expectReload` / `getStringMessage` / `getMostRecentHmrChunk` (all hardcoded 1000 ms), and raise the per-test base accordingly. Also make `waitForLine` scan already-buffered lines via the previously-dead `cursor` field so an `await` between stream creation and the call can't drop the match. ### Underlying DevServer bugs found while stress-testing - **`IncrementalGraph.invalidate` use-after-poison**: the incoming `path` (a slice into `HotReloadEvent.extra_files`) was stored in `entry_points`, but the event is reset — and its `extra_files` may be reallocated by the watcher thread — before `entry_points` is consumed by `startAsyncBundle` / `TestingBatch`. Since `getIndex(path)` already succeeded, store the graph-owned `keys[index]` instead. - **`TestingBatch.append`** stored the same borrowed slices as persistent keys across multiple `HotReloadEvent.run` calls. Dupe keys on insert; free them in `TestingBatch.deinit`. - **`onFileUpdate` (Linux)** indexed only `changed_files[event.name_off]` for a merged directory `WatchEvent`. When an atomic-save editor (vim/emacs/IntelliJ) lands `CREATE tmp` + `MOVED_TO target` in one coalesced inotify batch, the rename target was dropped and never re-watched. Forward every name via `event.names()`. ### Harness robustness - `waitForHotReload` used `clientWaits === connectedClients.size`; straggler HMR events from prior unsynchronized writes could push the count past, so it never matched. Use `>=`. - `waitForHotReload` now rejects on dev-server panic instead of hanging to the test timeout. - Detect `AddressSanitizer` / `ThreadSanitizer` / `==ABORTING` in subprocess output as a panic. ## How verified - New `hot.test.ts` case floods a watched directory (32 decoy creates + unlink + rename-over) to force inotify coalescing: - **without** `src/` changes → 3/3 fail under ASAN (use-after-poison in `TestingBatch.append` via `wyhash`) - **with** `src/` changes → 10/10 pass - `dev-and-prod.test.ts -t "rapid consecutive edits"` → 10/10 pass - Full runs of `hot`, `dev-and-prod`, `bundle`, `css`, `html`, `esm`, `stress`, `ssg-pages-router`, `incremental-graph-edge-deletion`, `plugins`, `sourcemap`, `server-sourcemap`, `vfile`, `framework-router`, `deinitialization` → all green (esm-11 is a pre-existing `skip: ["ci"]`) - `zig:check-all` passes on all targets Supersedes oven-sh#29575 and oven-sh#28211. Fixes oven-sh#19732 --------- Co-authored-by: robobun <robobun@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
May 5, 2026
…en-sh#30196) ## What does this PR do? Fixes a use-after-free in `HTMLRewriter.transform()` that caused flaky SIGSEGV crashes found by fuzzing. When transforming a string or ArrayBuffer, the body is buffered synchronously and fed to lol-html via `write()` followed by `end()`. If a document/element handler returns a rejected promise for the final `lastInTextNode` chunk (emitted from `end()`), the `end() catch` branch in `BufferOutputSink.runOutputSink` would call `response.finalize()` directly on the output `Response`. That `Response` is already owned by its JS wrapper cell (created earlier in `init()` via `sink.response.toJS()`), so destroying it in-place left the wrapper's `m_ctx` pointing at freed memory. When GC later swept the wrapper, its destructor invoked `Response.finalize()` again on that freed pointer: ``` AddressSanitizer: use-after-poison #0 bun.js.bindings.JSRef.JSRef.deinit src/bun.js/bindings/JSRef.zig:188 #1 bun.js.bindings.JSRef.JSRef.finalize src/bun.js/bindings/JSRef.zig:200 #2 bun.js.webcore.Response.finalize src/bun.js/webcore/Response.zig:474 #3 ResponseClass__finalize codegen/ZigGeneratedClasses.zig:17250 #4 WebCore::JSResponse::~JSResponse() codegen/ZigGeneratedClasses.cpp:54979 ``` The `write()` error path (just above it) already handled this correctly by returning the error and letting the JS wrapper own the Response lifetime. This PR makes the `end()` error path do the same — drop the manual `response.finalize()` and `sink.response = undefined`. ## How did you verify your code works? Minimal repro that reliably triggers the ASAN error before the fix and passes cleanly after: ```js const rewriter = new HTMLRewriter(); rewriter.onDocument({ text(chunk) { if (chunk.lastInTextNode) { return Promise.reject(new Error("boom")); } }, }); try { rewriter.transform(new Uint8Array([97, 98, 99]).buffer); } catch (e) {} Bun.gc(true); ``` Added regression tests in `test/js/workerd/html-rewriter.test.js` covering both ArrayBuffer and string inputs. All existing HTMLRewriter tests pass. --------- Co-authored-by: robobun <robobun@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
May 5, 2026
…0174) ## What `RequestContext` stored `response_ptr: ?*Response` and, for plain `Blob`/`InternalBlob`/`WTFStringImpl` bodies, left the Response JSValue unprotected. `renderBytes()` → `tryEnd()` can hit backpressure and register an `onWritable` callback, unwinding with `response_ptr` still set. Nothing rooted the Response (`RequestContext` is a pool struct, not GC-visited), so GC could finalize it. If the client then aborted while the request body was still `.Locked`, `onAbort()` dereferenced a freed `*Response` — heap-use-after-free under ASAN at `RequestContext.zig:692`. ## Repro ``` POST → handler returns new Response(8MB string) sync → tryEnd() backpressure (client paused) → onWritable registered, return → Bun.gc(true) → Response collected, response_ptr dangles → client.destroy() → onAbort → deref response_ptr → UAF ``` ASAN trace (unpatched): ``` ==ERROR: AddressSanitizer: use-after-poison #0 bun.js.bindings.JSRef.JSRef.tryGet #1 bun.js.webcore.Response.getBodyReadableStream #2 RequestContext.onAbort src/bun.js/api/server/RequestContext.zig:693 #3 uWS::HttpContext<false>::onClose ``` ## Fix Give `Response` a `weak_ptr_data` field (mirroring `Request.WeakRef`) and replace `response_ptr: ?*Response` with `response_weakref: Response.WeakRef` via `bun.ptr.WeakPtr`. `Response.destroy()` now defers freeing the allocation until outstanding weak refs drop; `WeakRef.get()` returns null once the contents are gone. `onAbort` / `handleResolveStream` / `handleRejectStream` call `.get()` and simply skip the readable-stream cleanup when it's null — a no-op for in-memory bodies anyway, since the body was already extracted via `useAsAnyBlobAllowNonUTF8String()` before backpressure. File-backed and `.Locked` bodies continue to `protect()` `response_jsvalue` as before; those paths need the Response's status/headers alive across the async hop for `renderMetadata()`. The hot path (small in-memory responses) no longer needs `protect()`/`unprotect()`. The two redundant `ctx.response_ptr = response` assignments right before `ctx.render(response)` are dropped — `render()` already sets the weak ref. ## Verification `test/js/bun/http/serve-response-gc-backpressure-abort.test.ts` (ASAN/debug-only): POST with incomplete chunked body so `request_body` stays `.Locked`, handler returns a large string Response, client pauses so `tryEnd()` stalls, `Bun.gc(true)` loop, then client closes. - **without fix**: `AddressSanitizer: use-after-poison` in `onAbort` → `Response.getBodyReadableStream` - **with fix**: passes, `abortCount === iterations`, `pendingRequests === 0` --------- Co-authored-by: robobun <robobun@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
May 5, 2026
…before write (oven-sh#30155) ## Repro ```js Bun.serve({ port: 0, fetch: () => new Response("hello", { headers: [ ["Transfer-Encoding", "gzip"], ["Transfer-Encoding", "chunked"], ], }), }); // HEAD / → ASAN heap-use-after-free in uWS::HttpResponse::writeHeader ``` The duplicate entries make `FetchHeaders` combine them via `makeString()`, producing a `StringImpl` held only by the header map — the minimal condition for the free to actually happen. StringImpl is allocated via bmalloc which ASAN doesn't instrument by default; with `Malloc=1` (bmalloc → system heap) the debug build reports: ``` AddressSanitizer: heap-use-after-free READ of size 13 #2 uWS::HttpResponse<false>::writeHeader #5 doRenderHeadResponse RequestContext.zig:1378 freed by: oven-sh#23 HTTPHeaderMap::remove oven-sh#28 doWriteHeaders RequestContext.zig:2303 oven-sh#29 renderMetadata RequestContext.zig:2209 oven-sh#30 doRenderHeadResponse RequestContext.zig:1377 ``` ## Cause `doRenderHeadResponse()` calls `headers.fastGet(.TransferEncoding)`, which returns a `ZigString` that **borrows** the header map entry's `StringImpl` bytes (no ref taken). For an ASCII value, `toSlice()` also borrows rather than copying. It then calls `this.renderMetadata()`, whose `doWriteHeaders()` does `headers.fastRemove(.TransferEncoding)` (and `renderMetadata` also `swapInitHeaders()` + `deref()`s the whole `FetchHeaders`). When the map held the only reference to the `StringImpl`, it's destroyed right there — and the very next line `resp.writeHeader("transfer-encoding", transfer_encoding_str.slice())` writes the freed bytes to the socket. The adjacent `Content-Length` branch has the same bug: `std.fmt.parseInt()` runs on the borrowed slice *after* `renderMetadata()` has already `fastRemove(.ContentLength)`'d it. ## Fix - **Transfer-Encoding**: use `toSliceClone()` instead of `toSlice()` so the value is owned and survives `renderMetadata()`. - **Content-Length**: parse the integer *before* `renderMetadata()` (and drop the slice immediately), so the borrowed bytes are never touched after the header entry is removed. No extra allocation needed since only the parsed `usize` is used afterwards. ## Verification New test in `test/js/bun/http/bun-server.test.ts` (inside the existing `HEAD requests oven-sh#15355` block) spawns a subprocess with `Malloc=1` (non-Windows), serves HEAD responses whose Transfer-Encoding / Content-Length values are `makeString()`-combined (sole-owner StringImpl), and asserts the raw wire output. ``` git stash push -- src/ → test fails with "AddressSanitizer: heap-use-after-free" in stderr git stash pop → test passes ``` All other tests in the `HEAD requests oven-sh#15355` describe block continue to pass. Co-authored-by: robobun <robobun@users.noreply.github.com>
github-actions
Bot
force-pushed
the
deps/update-sqlite
branch
from
May 10, 2026 08:11
66ff949 to
4d0cd3b
Compare
springmin
pushed a commit
that referenced
this pull request
May 17, 2026
…s, http (oven-sh#30722) Hardens 36 reachable security findings across the runtime, package manager, parsers, HTTP client/server, and SQL drivers. Three auto-applied fixes (oven-sh#61 SSL exception leak, oven-sh#68 YAML merge dedup, oven-sh#104 archive overwrite precheck) were dropped: oven-sh#61 introduced a use-after-free, oven-sh#68 stored a non-`'static` byte view in a `'static` field, and oven-sh#104 added dead gating that did not close the traversal. ### Memory safety / lifetime - #2 — Dangling proxy slice across reentrant JS getter — copy `process.env` proxy href to an owned `Vec` before reentrant getters can free the env map (`Blob.rs`) - #15 — Rollback restores dangling editor name pointer — preserve and restore `name_storage` on `detect_editor` failure (`BunObject.rs`) - oven-sh#81 — Reentrant reconnect frees live handlers — only free previous handlers when `active_connections == 0` (`Listener.rs`) - oven-sh#110 — Async randomFill uses stale resizable buffer pointer — fill a worker-owned scratch buffer; copy back on the JS thread after re-validating bounds (`node_crypto_binding.rs`) - oven-sh#119 — Null zero-length slice UB in DOMJIT fast path — use `ffi::slice` which tolerates `(null, 0)` (`Crypto.rs`) - oven-sh#67 — Raw serialization reads struct padding bytes — add explicit `_padding_*` fields with `offset_of!` proof asserts (`npm.rs`) - oven-sh#74 — TLS rejection path leaks websocket refcount — route SSL/auth failures through `self.fail()` which clears `outgoing_websocket` (`websocket_client.rs`) - oven-sh#108 — FD-backed fetch body leaks duplicated descriptor — close `opened_fd` unconditionally after `read_file` (`fetch.rs`) ### Untrusted-input bounds / panics - #10 — Invalid lockfile tag causes panic DoS — replace `unreachable!()` with logged error + `Tag::Uninitialized` (`dependency.rs`) - oven-sh#20 — Unchecked lockfile string offsets cause OOB slice — bounds-check non-inline `String` pointers against `ctx.buffer` (`dependency.rs`) - oven-sh#91 — Panic on unvalidated resolution tag — validate `ResolutionTag` discriminants on lockfile load (`Package.rs`) - oven-sh#24 — Unwrap panic on unexpected 304 response — return `UnexpectedNotModified` when no cached manifest exists (`npm.rs`) - oven-sh#44 — UDP port getter unwrap panic on transient state — return `undefined` when `socket` is `None` (`udp_socket.rs`) - oven-sh#36 — Close reason length mismatch causes panic — clamp `body_len` to 125 and bail on overlong UTF-8 transcode (`websocket_client.rs`) - oven-sh#100 — Windows pipe name length panic DoS — `debug_assert` → real bounds check (`Listener.rs`) - oven-sh#60 / oven-sh#111 — Windows shim stack buffer overflows — bounds-check argument and filename writes against `BUF1_LEN`/`BUF2_U16_LEN` before `copy_nonoverlapping` (`bun_shim_impl.rs`) - oven-sh#76 / oven-sh#101 — Unchecked bin name/entry name copies — bounds-check before slicing into `abs_dest_buf` (`bin.rs`) - oven-sh#79 — `if` keyword misclassification causes parser panic — require a delimiter token before classifying (`shell_parser/parse.rs`) - oven-sh#32 — Bounds check occurs after UTF-16 write — pre-flight key/value lengths before `convert_utf8_to_utf16_in_buffer` (`env_loader.rs`) - oven-sh#95 — PBKDF2 digest validation allows panic-only algorithm — reject digests with no `EVP_MD` (`PBKDF2.rs`) ### DoS / resource caps - oven-sh#17 — Unbounded recursion on deep TOML dotted keys — cap dotted-key segments at 512 (`toml.rs`) - oven-sh#39 — Unbounded brace expansion preallocation — cap expansion count at 65536 in `Bun.$` and `Bun.braces` (`BunObject.rs`, `Expansion.rs`) - oven-sh#31 — SCRAM PBKDF2 parameters accepted from server — clamp iteration count to `[4096, 10M]`, salt length to `[1, 1024]` (`PostgresSQLConnection.rs`) ### Auth / injection / traversal - oven-sh#19 — Cleartext password sent after TLS downgrade — require `TLSStatus::SslOk`, not just `ssl_mode != Disable` (`MySQLConnection.rs`) - oven-sh#83 — Strict TLS request reuses lax-verified pooled socket — track `established_with_reject_unauthorized` and refuse pool reuse for strict callers (`HTTPContext.rs`, `lib.rs`, `ClientSession.rs`) - oven-sh#73 — IPv6 loopback prefix auth bypass — exact-match `::1` instead of `starts_with` (`server_body.rs`) - oven-sh#56 — Unsanitized filename injects response headers — reject `\r`/`\n`/NUL/`"` in `content-disposition` filenames (`RequestContext.rs`) - oven-sh#43 — Missing CRLF checks for signed host/auth headers — also validate `region`, `access_key_id`, and `host` (`s3_signing/credentials.rs`) - oven-sh#34 — Bucket slash enables S3 host confusion — reject buckets containing `/` (`s3_signing/credentials.rs`) - oven-sh#25 — Lexical symlink check permits extraction escape — track created symlinks during extraction and refuse paths that traverse them (`libarchive/lib.rs`) - oven-sh#71 — bunx executes untrusted temp-cache binary — `lstat` cached binary; refuse symlinks and other-uid files (`bunx_command.rs`) ### Permission hygiene - #6 — Bin target chmod always sets mode 0777 — `0o777 & !umask` instead of `umask | 0o777` (`bin.rs`) - oven-sh#23 — Process umask cleared and never restored — restore umask after probing it in `ensure_umask` (`bin.rs`) ### Parser correctness - oven-sh#22 — Sign-prefixed scalar misparsed as infinity — fix Zig→Rust `&&`/`||` precedence transliteration (`yaml.rs`)
springmin
pushed a commit
that referenced
this pull request
May 19, 2026
… stack pointer (oven-sh#31020) ## What `resolveMaybeNeedsTrailingSlash` swaps `vm.log` / `resolver.log` to a stack-local `Log` for the duration of `_resolve`, then restores them via a drop guard. The Zig original also swaps and restores `transpiler.linker.log` and `resolver.package_manager.log`; the Rust port had those behind a `TODO(b2-cycle)` and only handled `vm.log` + `resolver.log`. When auto-install is enabled and the resolver lazily creates the `PackageManager` during `_resolve`, `Resolver::get_package_manager` seeds `pm.log` from `resolver.log` — which at that point is the **stack-local** `Log`. Because the restore guard never touched `pm.log`, it was left pointing into a dead stack frame after the function returned. The next resolve at a different stack depth that routes through the auto-install task runner dereferenced that stale pointer in `Log::add_error_fmt`, tripping ASAN's `stack-use-after-scope` (or segfaulting / executing garbage in release builds). Stack at the fault: ``` #0 bun_ast::Log::add_formatted_msg #1 bun_ast::Log::add_error_fmt #2 bun_install::…::run_tasks #7 bun_install::…::enqueue_dependency_to_root #9 bun_resolver::Resolver::enqueue_dependency_to_resolve #14 bun_resolver::Resolver::resolve_and_auto_install #15 bun_jsc::VirtualMachine::_resolve oven-sh#16 bun_jsc::VirtualMachine::resolve_maybe_needs_trailing_slash::<true> ``` ## Fix Swap and restore `linker.log` and (when present) `package_manager.log` in both copies of the resolve log guard (`VirtualMachine::resolve_maybe_needs_trailing_slash` and `jsc_hooks::resolve_hook`), matching `VirtualMachine.zig`. The restore re-checks `resolver.package_manager` at drop time so a PM that was lazily created during `_resolve` is also pointed back at the VM log. Also adds the missing `<cassert>` include in `wtf-bindings.cpp`, which stopped being pulled in transitively. ## Repro ```js // run from an empty dir with // BUN_CONFIG_INSTALL=fallback BUN_CONFIG_REGISTRY=http://127.0.0.1:1 const realm = new ShadowRealm(); const variants = [ () => realm.importValue("pkg-not-found-a", "x"), () => (() => realm.importValue("pkg-not-found-b", "x"))(), () => (() => (() => realm.importValue("pkg-not-found-c", "x"))())(), () => import("pkg-not-found-f"), ]; for (let i = 0; i < 100; i++) for (const v of variants) try { v()?.catch?.(() => {}); } catch {} ``` Segfaults on `main`, clean after this change. Fixes oven-sh#14432 Fixes oven-sh#22407 --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
May 23, 2026
…s longer than the comparand (oven-sh#31264) ### What does this PR do? Fixes an ASAN `global-buffer-overflow` found by fuzzing the CSS parser: ``` asan:global-buffer-overflow:strncasecmp|eql_case_insensitive_ascii|eql_case_insensitive_ascii|bun_core::string::immutable::eql_case_insensitive_ascii_ignore_length ``` **Repro** ```sh BUN_FEATURE_FLAG_INTERNAL_FOR_TESTING=1 bun -e 'require("bun:internal-for-testing").cssInternals.minifyTest(":nth-child(Nn", "")' ``` ``` ==ERROR: AddressSanitizer: global-buffer-overflow READ of size 2 ... #0 strncasecmp #1 bun_core::strings_impl::eql_case_insensitive_ascii src/bun_core/lib.rs #2 bun_core::string::immutable::eql_case_insensitive_ascii_ignore_length src/bun_core/string/immutable.rs #3 bun_css::css_parser::nth::parse_nth src/css/css_parser.rs #4 bun_css::selectors::parser::parse_nth_pseudo_class src/css/selectors/parser.rs ``` **Cause** `strings_impl::eql_case_insensitive_ascii(a, b, check_len)` defers to `strncasecmp(a, b, a.len())`, which reads up to `a.len()` bytes from *both* buffers. The Zig original (`strings.eqlCaseInsensitiveASCII`) compared against NUL-terminated comptime literals, so `strncasecmp` stopped at the sentinel and reported a mismatch whenever `a` was longer than `b`. Rust byte-string literals carry no terminator, so the An+B parser's ident branch (`parse_nth`), which compares an arbitrary user ident against the keywords `"even" / "odd" / "n" / "-n" / "n-" / "-n-"` with the ignore-length variant, reads past the end of the keyword literal as soon as the ident is longer than the keyword and shares its prefix (`Nn` vs `n`, `n-3` vs `n`, …). Besides the OOB read, the comparison result depended on whatever byte happens to follow the literal in rodata. **Fix** Reject `b.len() < a.len()` up front in `eql_case_insensitive_ascii` before calling `strncasecmp` — the same result the NUL sentinel produced in Zig, so observable behavior is unchanged for every in-bounds input (all other callers of the ignore-length variant already pass equal-length slices). `strncasecmp` now only ever reads within both slices. **Verification** - `bun bd test test/js/bun/css/nth-anplusb-ident.test.ts` without the fix (src/ stashed): aborts with the ASAN global-buffer-overflow above. - With the fix: passes. The new test covers valid `n-<digits>` idents that are longer than the `n`/`n-` keywords (`:nth-child(n-3)`, `:nth-child(N-3)`, `:nth-last-child(n- 42)`), keyword case-insensitivity (`:nth-child(N)`), an invalid ident (`:nth-child(NN)` → parse error), and the exact fuzzer-minimized input run in a subprocess. - `bun bd test test/js/bun/css/css.test.ts`: 1032 pass, 0 fail (no behavior change for the existing suite). - A second fuzz report hits the same overflow through `Bun.build` with a CSS entrypoint containing `:nth-child(Nn`; that path goes through the same `parse_nth` comparison and is covered by this fix (`Bun.build` now reports a parse error instead of aborting). - The `build-rust` CI failures on this PR (unused label / unnecessary `unsafe` warnings in `src/spawn`, `src/install`, `src/crash_handler`, `src/runtime/ffi`, `src/runtime/dns_jsc`) are present on current `main` commits that don't include this change and come from files this PR doesn't touch.
springmin
pushed a commit
that referenced
this pull request
Jun 18, 2026
…letes mid-read (oven-sh#31959) [publish images] Fixes a use-after-free in the HTTP client's proxy tunnel close path (Sentry BUN-2VY8, ~10 events/day on Windows release builds; reproduces deterministically under ASAN on all platforms). ## Repro `fetch()` through an HTTP CONNECT proxy to an HTTPS origin, where the origin's final response bytes and its TLS `close_notify` reach the client in a single TCP batch (origin writes the response and immediately closes). The regression test builds exactly that: a local CONNECT proxy that holds origin-to-client bytes after the handshake and flushes session tickets + response + close_notify in one write. On an unfixed ASAN build: ``` ERROR: AddressSanitizer: heap-use-after-free READ of size 8 thread T11 (HTTP Client) #0 Option<RefPtr<ProxyTunnel>>::as_ref #1 bun_http::proxy_tunnel::on_close src/http/ProxyTunnel.rs:525 #2 SSLWrapper<*mut HTTPClient>::trigger_close_callback src/uws/lib.rs:802 #3 SSLWrapper<*mut HTTPClient>::handle_reading src/uws/lib.rs:1022 #4 SSLWrapper<*mut HTTPClient>::handle_traffic #5 SSLWrapper<*mut HTTPClient>::receive_data #6 ProxyTunnel::receive src/http/ProxyTunnel.rs:751 freed by: AsyncHTTP::on_async_http_callback_raw src/http/AsyncHTTP.rs:813 HTTPClient::send_progress_update_without_stage_check src/http/lib.rs:3793 ``` ## Cause 1. `handle_reading` processes the batch: `SSL_read` returns the body bytes, the next `SSL_read` hits `close_notify` (`SSL_ERROR_ZERO_RETURN`), which sets `received_ssl_shutdown` and `sent_ssl_shutdown` before flushing the already-decrypted bytes through the data callback. 2. The data callback completes the response. The done path runs `close_proxy_tunnel(true)` -> `ProxyTunnel::shutdown()` -> `SSLWrapper::shutdown(true)`, which hits the already-shut-down early return (`sent_ssl_shutdown || fatal_error`) and returns **without setting `closed_notified`**. The result callback then frees the `ThreadlocalAsyncHTTP` embedding the `HTTPClient`, the exact pointer stored in the wrapper's `handlers.ctx`. 3. Control returns to `handle_reading`. Its liveness guard (`ssl.is_none() || closed_notified()`) passes because neither is set, so `trigger_close_callback()` invokes `on_close(handlers.ctx)` on the freed client. When the allocation has been recycled, `on_close` can ref or close a different request's tunnel instead of faulting. ## Fix `src/uws/lib.rs`: when `SSLWrapper::shutdown(fast_shutdown=true)` takes the already-shut-down early return, fire `trigger_close_callback()` (idempotent via `closed_notified`) so the wrapper is marked closed before the owner detaches and frees `handlers.ctx`. A fast shutdown is a full teardown, and the normal fast-shutdown path already fires the close callback unconditionally; this only closes the gap where the SSL-level shutdown had already happened. Graceful `shutdown(false)` (node:tls half-close via UpgradedDuplex / WindowsNamedPipe) is unchanged, so reads after a sent `close_notify` keep working. ## Verification New test in `test/js/bun/http/proxy.test.ts` (`test.skipIf(!isASAN)`, the UAF is only deterministic under ASAN): fails on an unfixed ASAN debug build with the heap-use-after-free above, passes with the fix. Full `proxy.test.ts` (46 tests) plus `node-tls-connect`, `node-tls-upgrade`, `node-tls-duplex-close-throw-uaf`, `node-tls-socket-allow-half-open-option`, `node-tls-server`, `fetch-tls-cert`, and `node-https-checkServerIdentity` suites pass. ## Note on the asan-lane CI failure (oven-sh#32144) The intermittent LeakSanitizer failure on the x64-asan shard (deferred napi finalizers parked on a never-drained cleanup-hook list at `bun test` exit) is being fixed in oven-sh#32146, which carries the same `global_exit()` drain plus a hooks-only guard that skips pending `napi_wrap` finalizers on undrained-loop exits. A subset version of that fix was briefly on this branch (e59bc1d) but without the hooks-only guard it made `test/js/third_party/duckdb/duckdb-basic-usage.test.ts` SEGV at exit on the asan lane (build 62135), exactly the failure mode oven-sh#32146's guard prevents, so it was reverted (61f9e70). This PR is scoped to the proxy-tunnel UAF; its asan lane can still intermittently hit the pre-existing oven-sh#32144 leak until oven-sh#32146 lands. ## Related PRs - oven-sh#30606 addresses the same crash signature but patches only the `.zig` reference files, which are no longer compiled; this PR fixes the shipping Rust implementation. - oven-sh#31952 fixes the same UAF by calling a new `mark_close_notified()` helper from `ProxyTunnel::shutdown` (silently setting the flag at one call site, with `close_raw` exempted). This PR instead closes the gap inside `SSLWrapper::shutdown(true)` itself, so every fast-shutdown caller (`ProxyTunnel::shutdown`, `ProxyTunnel::close_raw`, `UpgradedDuplex::close`, `WebSocketProxyTunnel::shutdown`) gets the same "no callbacks after teardown" guarantee without new wrapper API or a shutdown/close_raw asymmetry. The close callback is fired rather than suppressed, so the error teardown path keeps delivering `on_close` -> `close_and_fail` exactly once (idempotent via `closed_notified`). Test here is a deterministic single-shot repro (the test proxy reassembles TLS records and flushes tickets + response + close_notify in one write) rather than an iteration loop. --------- Co-authored-by: Ciro Spaciari MacBook <ciro@anthropic.com>
springmin
pushed a commit
that referenced
this pull request
Jun 23, 2026
…e re-enters the event loop (oven-sh#32597) Sentry BUN-2WJA / BUN-2WKB (~290 events combined, Windows x86_64, `http_server=True`, bun 1.2.23 through 1.3.14): ``` Segmentation fault at address 0xFFFFFFFFFFFFFFFF endWithSink src/runtime/webcore/Sink.zig:577 endFromJS src/runtime/webcore/streams.zig:1200 finalize src/runtime/webcore/streams.zig:1301 clearAndFree src/collections/baby_list.zig:148 memset (fault at 0xFFFFFFFFFFFFFFFF) ``` ## Cause The generated `JSReadable*Controller` `end()` and `close()` host functions (`src/codegen/generate-jssink.ts`) stash `m_sinkPtr` in a local, call `controller->detach()`, and only afterward dereference the stashed pointer via `endWithSink()` / `${name}__close()`: ```cpp void *ptr = controller->wrapped(); controller->detach(); // runs onClose JS synchronously return ${name}__endWithSink(ptr, lexicalGlobalObject); // derefs ptr ``` `detach()` invokes the stored `onClose` callback. For a `type: "direct"` stream this is `readDirectStream`'s `close(stream, reason)`, which calls `underlyingSource.cancel()`. That is arbitrary user code running while `ptr` is still live on the C++ stack. If the stream's `pull()` promise has already settled, `RequestContext::on_resolve_stream` is sitting in the microtask queue. Any path from `cancel()` that drains microtasks (e.g. the server-side drain points in `on_response` / `do_render_with_body`, or an explicit `drainMicrotasks()`) runs `handle_resolve_stream`, which calls `destroy_sink` and frees the `HTTPServerWritable`. `endWithSink(ptr)` then enters `end_from_js` on the freed allocation; `finalize()` reads garbage for `pooled_buffer` / `buffer.cap` / `buffer.ptr` and faults in the `memset` the allocator's free-scrub path performs. The same ordering appears in the Rust port (`streams.rs` / `Sink.rs`) unchanged. ## Fix In `${controller}__end` and `${controller}__close`, finish the native sink operation before any JS runs: 1. Call `${name}__controllerDetached(ptr, controller)` and null `m_sinkPtr` up front (so `end_from_js`'s own `signal.close()` stays a no-op, matching the previous behaviour, and so the later `detach()` won't touch the native side again). 2. Run `endWithSink(ptr)` / `close(ptr)`. 3. Call `controller->detach()` last. With `m_sinkPtr` already null it only clears `m_onPull` and fires `onClose`; by now we hold no reference into the sink, so re-entrant teardown is safe. ## Verification New ASAN-gated test in `test/js/bun/http/serve-direct-readable-stream.test.ts` reproduces the exact UAF deterministically by draining microtasks from the stream's `cancel()` callback (the test uses `require("bun:jsc").drainMicrotasks()` to force the drain that the production crash hits via the server's own drain points). <details> <summary>ASAN output on the unfixed build</summary> ``` ==22203==ERROR: AddressSanitizer: heap-use-after-free on address 0x6ee5f87602ca READ of size 1 at 0x6ee5f87602ca thread T0 #0 HTTPServerWritable::end_from_js src/runtime/webcore/streams.rs:1831 #2 JSSink::js_end_with_sink src/runtime/webcore/Sink.rs:1107 #4 WebCore::JSReadableHTTPResponseSinkController__end JSSink.cpp:620 freed by thread T0 here: #10 HTTPServerWritable::destroy src/runtime/webcore/streams.rs:1950 #11 RequestContext::destroy_sink src/runtime/server/RequestContext.rs:1930 #12 RequestContext::handle_resolve_stream src/runtime/server/RequestContext.rs:2680 #13 RequestContext::on_resolve_stream src/runtime/server/RequestContext.rs:2716 ... oven-sh#24 JSC::VM::drainMicrotasks() ``` </details> With the fix the fixture completes normally. Existing suites (`serve.test.ts`, `bun-server.test.ts`, `direct-readable-stream.test.tsx`, `streams.test.js`, the sink leak tests) show no new failures against the unfixed build. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
Jun 27, 2026
…oven-sh#32742) ### What does this PR do? Fixes a use-after-free in the HTTP client's CONNECT proxy tunnel, caught by ASAN: ``` READ of size 8 at 0x61e00001fe80 thread T6 #0 Option<RefPtr<ProxyTunnel>>::as_ref #1 proxy_tunnel::on_close ProxyTunnel.rs:525 #2 SSLWrapper::trigger_close_callback uws/lib.rs:833 #3 SSLWrapper::handle_reading uws/lib.rs:1053 ... freed by thread T6 here (same stack, same `handle_reading` call): #5 AsyncHTTP::on_async_http_callback_raw AsyncHTTP.rs:819 #7 HTTPClient::send_progress_update_without_stage_check #9 proxy_tunnel::on_data ProxyTunnel.rs:350 #11 SSLWrapper::trigger_data_callback uws/lib.rs:824 #12 SSLWrapper::handle_reading uws/lib.rs:1046 ``` `SSLWrapper::handle_reading` flushes pending decrypted bytes to the data callback, then runs the close callback, guarded only by `closed_notified`: 1. The flushed data callback completes a keep-alive response through the tunnel. A fatal TLS record error sets only `fatal_error` — none of the shutdown flags — so the wrapper passed `tunnel_poolable`'s `!is_shutdown()` check and the tunnel was handed to the keep-alive pool. Nothing called `wrapper.shutdown()`, so `closed_notified` was never latched. Dispatching the final result then freed the `ThreadlocalAsyncHTTP` that embeds the `HTTPClient`. 2. The guard (`ssl.is_none() || closed_notified()`) passes. 3. `trigger_close_callback()` invokes `on_close(handlers.ctx)` with `ctx` pointing at the freed client. The pooling branch is the only terminal path that doesn't go through `close_proxy_tunnel(true)` → `wrapper.shutdown()` → `closed_notified`, which is the latch the read loop relies on. `SSLWrapper::shutdown` already special-cases the *close_notify* flavor of this for exactly that reason; the fatal-error flavor never reaches `shutdown()`. The fix is one predicate: a tunnel whose wrapper has a fatal error or pending unconsumed input/output is not poolable. That routes it through the orderly teardown that latches `closed_notified`, and the pending-I/O half closes the same hole for a tunnel pooled from a mid-loop data callback while more decrypted bytes or queued output remain. Both are also required for the pool to be correct on its own terms — a poisoned or dirty TLS session must not be handed to the next request. ### How did you verify your code works? New regression test in `test/js/bun/http/proxy.test.ts` (next to the existing close_notify sibling): an HTTPS keep-alive response through a CONNECT proxy with a corrupt TLS record appended to the same TCP burst, followed by a second request that can only complete if the HTTP client thread survived the first. Against an unfixed ASAN debug build the fixture aborts every run: ``` ==20981==ERROR: AddressSanitizer: heap-use-after-free on address 0x61e00001fe80 READ of size 8 at 0x61e00001fe80 thread T6 ... exit=134 ``` With this change it prints `4096 200 200` and exits 0 with no ASAN report. `test/js/bun/http/proxy.test.ts` (49/49), `fetch-proxy-connect-tunnel-split-envelope.test.ts`, `fetch-proxy-tls-intern-race.test.ts`, and `fetch-keepalive.test.ts` all pass.
springmin
pushed a commit
that referenced
this pull request
Jun 27, 2026
…oven-sh#32743) ## What `ReadableStream::from_pipe` (the `proc.stdout` / `proc.stderr` path for `Bun.spawn` and the shell subprocess) moves an already-registered pipe poll from the subprocess `PipeReader` into a freshly allocated `NewSource<FileReader>` and re-points the poll's owner at it. The across-read ref that keeps that box alive (`waiting_for_on_reader_done` + `increment_count()`, which upgrades `this_jsvalue` to `Strong`) was only taken in `FileReader::on_start`, i.e. the first time JS actually pulls from the stream. Between `from_pipe` and that first pull, the poll's owner points into a box whose only ref is the JS wrapper's own `Weak` back-reference. If the `Subprocess` and its cached stdout become unreachable before anyone pulls (a fire-and-forget spawn where `proc.stdout` is touched but never read, and the direct child exits while something else still holds the write end), GC sweeps the `JSFileInternalReadableStreamSource` wrapper and frees the `NewSource<FileReader>` box while the poll is still armed. The next readability or EOF event dispatches into freed memory: ``` READ of size 8 (heap-use-after-free) #0 Vec::len / is_empty (freed Vec<u8>) #2 webcore::file_reader::FileReader::on_reader_done FileReader.rs:1008 #3 bun_io::pipe_reader::read_socket{closure} PipeReader.rs:846 #4 PosixBufferedReader::read_socket PipeReader.rs:576 #5 file-poll dispatch <- posix_event_loop <- us_internal_dispatch_ready_polls freed by: JSC::JSDestructibleObjectDestroyFunc <- MarkedBlock sweep <- MarkedSpace::sweepBlocks allocated: ReadableStream::from_pipe<subprocess::PipeReader> -> NewSource<FileReader> ``` Found by a coverage-guided GC-stress fuzzer with syscall interposition (`BUN_JSC_collectContinuously=1` plus an injected `EAGAIN` to keep the read pending). In release builds this is silent heap corruption. ## Fix Take the across-read ref in `from_pipe` itself, immediately after the live reader is transferred and the JS wrapper is created, so the box is `Strong`-rooted for as long as the poll can fire. `on_reader_done` / `on_reader_error` release it exactly as before. `FileReader::on_start` now checks `waiting_for_on_reader_done` before taking the ref so the later `handle.start()` call from `lazyLoadStream` does not double-count on this path. ## How did you verify your code works? The test asserts the lifetime invariant directly via `heapStats().objectTypeCounts.FileInternalReadableStreamSource` rather than racing for the crash, since the exact UAF trigger depends on the fuzzer's syscall interposition. A detached grandchild (`sh -c 'while [ ! -e FLAG ]; do sleep 0.02; done; echo x'`) inherits the child's stdout and keeps the write end open past the direct child's exit, so the `FileReader`'s poll is still armed while we force GC with nothing in JS referencing the wrapper. - **Before** (`git stash push -- src/` + `bun bd test`): `duringLivePipe = 0` of 4; every wrapper swept while its poll owner still points into the freed box. - **After**: `duringLivePipe >= 4`; once the grandchildren exit and the pipes EOF, `afterEof <= 1` (one may remain via a conservatively-rooted final `Subprocess`, same caveat as `spawn-ipc-gc.test.ts`). Also passes `spawn-streaming-stdout.test.ts`, `spawn-unread-stdout-gc.test.ts`, `spawn-ipc-gc.test.ts`, `spawn-stdout-iterate-leak.test.ts`, and `readablestream-helpers.test.ts`. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
Jun 29, 2026
…ll-driven read (oven-sh#32986) ## Problem Heap use-after-free in Bun Shell when `epoll_ctl` fails while re-registering a pipe's `FilePoll` from a poll-driven read. Found by syscall-fault-injection fuzzing against `origin/main`. Follow-up to oven-sh#32754, which fixed the same failure on the eager spawn-time read path. ``` ERROR: AddressSanitizer: heap-use-after-free READ of size 1, thread T0 #0 <bun_io::pipe_reader::BufferedReaderVTable>::link io/PipeReader.rs:105 #1 <bun_io::pipe_reader::BufferedReaderVTable>::on_read_chunk io/PipeReader.rs:125 #2 <bun_io::pipe_reader::PosixBufferedReader>::read_with_fn io/PipeReader.rs:890 #3 <bun_io::pipe_reader::PosixBufferedReader>::read_socket io/PipeReader.rs:576 #4 <bun_io::pipe_reader::PosixBufferedReader>::on_poll io/PipeReader.rs:529 #5 __bun_run_file_poll runtime/dispatch.rs:677 freed by: <alloc::sync::Arc<bun_runtime::shell::subproc::PipeReader>>::drop ``` ## Repro 1. `PipeReader::start` registers the poll and the eager spawn-time `read_all()` hits `EAGAIN`, so `read_with_fn`'s `EAGAIN` arm re-registers the poll and the spawn returns. 2. The child writes to stdout and the poll fires. `__bun_run_file_poll`'s `BUFFERED_READER` arm dispatches straight into `PosixBufferedReader::on_poll` with a bare `&mut *h` and no keepalive. 3. `read_with_fn` drains the chunk, `recv()` returns a real `EAGAIN`, and `register_poll()` issues another `epoll_ctl`, which fails (`ENOMEM` in the repro). 4. `register_poll` dispatches `on_reader_error`. The shell `PipeReader::on_reader_error` signals the `Cmd`, the `Readable::Pipe` `Arc` is dropped, and the callback's own `guard_from_raw` keepalive becomes the last reference. The code already documents this: "Dropping `guard` is the matching `deref()`; may free `this`." 5. Back in `read_with_fn`, the `EAGAIN` arm still delivers the drained head: `parent.vtable.on_read_chunk(.., ReadState::Drained)` reads the freed vtable. Traced with the test's `LD_PRELOAD` shim: ``` [shim] epoll_ctl(ADD fd=13) unix call#1 -> ok PipeReader::start [shim] recv(fd=13) unix call#1 -> EAGAIN eager read, inside spawn [shim] epoll_ctl(MOD fd=13) unix call#2 -> ok re-register; spawn returns [shim] recv(fd=13) unix call#2 poll fired: the child's bytes [shim] recv(fd=13) unix call#3 real EAGAIN [shim] epoll_ctl(MOD fd=13) unix call#3 -> ENOMEM register_poll fails [shell_subproc] PipeReader(0x..250) onReaderError errno: 12 [shell_subproc] PipeReader(0x..250, stdout) detach() [shell_subproc] PipeReader(0x..250, stdout) deinit() ==ERROR: AddressSanitizer: heap-use-after-free ``` ## Cause `register_poll()`'s failure path dispatches `on_reader_error`, which the `BufferedReaderParent` contract explicitly allows to free the parent, but `register_poll` gave the caller no way to know that happened. `read_with_fn`'s `EAGAIN` arm is the only call site that touches the reader afterwards; every other `register_poll()` is in tail position. The `SAFETY` comment above the `parent` rebind claimed the parent is "never freed mid-call", which holds for `on_read_chunk` re-entry but not for `on_reader_error`. oven-sh#32754 covered this exact sequence on the eager spawn-time entry by holding an `Arc<PipeReader>` across `start()` and `read_all()` in `Readable::start_pipe_reader`. The epoll dispatch has no equivalent keepalive, so the poll-driven entry was still exposed. ## Fix `PosixBufferedReader::register_poll()` now returns whether registration succeeded. `false` means `on_reader_error` was dispatched and `self` must not be touched again, so `read_with_fn`'s `EAGAIN` arm returns there instead of delivering the drained head to a possibly freed parent. The stream has already been completed with the registration error at that point, so nothing is lost. All other `register_poll()` call sites are tail calls and discard the result. ## Test Two new modes in `test/js/bun/shell/shell-pipe-read-fault.test.ts`'s `LD_PRELOAD` fault shim: - `SHELL_RECV_EAGAIN_FIRST=1`: the first `recv()` on each `AF_UNIX` socket returns `EAGAIN`, pushing the first successful read off the eager spawn-time `read_all()` and onto the epoll dispatch. - `SHELL_FAIL_EPOLL_FROM=N`: the Nth and later `epoll_ctl` `ADD`/`MOD` on each `AF_UNIX` socket fail with `ENOMEM`. `N=3` lets the initial registration and the eager read's re-registration succeed, then fails the first poll-driven one. The new test is `skipIf(!isASAN)` because the use-after-free is only reliably observable under ASAN. With `src/io/PipeReader.rs` reverted to `main` it fails in ~1.1s with the `heap-use-after-free` above; with the fix all 6 tests in the file pass. ## Out of scope Shell `PipeReader::on_read_chunk` also calls `self.reader.register_poll()` from a `&mut self` method whose stated contract is that it never frees `self`. If that inner registration fails, the same free can happen under `read_with_fn`'s mid-loop flush instead of its `EAGAIN` arm. Reaching it needs a large (>32 KB) burst in one poll wake; I have not reproduced it, so it is not changed here.
springmin
pushed a commit
that referenced
this pull request
Jun 29, 2026
…low-priority queue (oven-sh#33006) ### Symptom AddressSanitizer reports a heap-use-after-free (READ of size 8 and WRITE of size 8 variants) in uSockets' listener bookkeeping while a TLS server accepts connections under load: ``` ==ERROR: AddressSanitizer: heap-use-after-free (WRITE of size 8) #0 us_internal_socket_group_unlink_socket bun-usockets/src/context.c:223 #1 us_internal_socket_close_raw bun-usockets/src/socket.c:291 #2 us_internal_ssl_close bun-usockets/src/crypto/openssl.c #3 close<true> src/uws_sys/socket.rs ``` A second manifestation site is the low-priority queue walker, `us_internal_handle_low_priority_sockets`. The trigger is an ordinary `Bun.serve({tls})` / `node:tls` server whose clients connect, handshake, and disconnect at inopportune times. No unusual client behavior is required. ### Cause uSockets throttles concurrent TLS handshakes. When the 5-per-tick budget runs out, the readable dispatch parks the socket in the loop-wide low-priority queue (`loop->data.low_prio_head`): it is unlinked from `group->head_sockets` and READABLE is removed from its poll. The two lists share the same `prev`/`next` fields, so a socket lives in exactly one at a time. A parked socket can still get a WRITABLE dispatch. When its handshake flight is backpressured (`send` returned short or 0), `us_internal_ssl_on_writable` retries the BIO write, and `us_socket_raw_write` unconditionally runs `us_poll_change(READABLE | WRITABLE)`. READABLE is now re-enabled on a socket that is still in the low-priority queue. The next readable dispatch on that socket, with the budget exhausted, parked it a second time. That path ran `us_internal_socket_group_unlink_socket(g, s)` on a socket whose `prev`/`next` are low-priority-queue links, not group links: - If the socket was the queue head, `group->head_sockets` gets pointed at the next low-priority socket. When that socket is later closed through `us_internal_socket_close_raw`'s low-priority branch, nothing repairs `head_sockets`, and the group list reaches freed memory. `us_internal_socket_group_unlink_socket`'s `next->prev = prev` for a neighbor is the WRITE of size 8. - `loop->data.low_prio_head` can be left pointing at the re-prepended socket as a self-cycle; the queue walker then reads through entries the close path has already freed. That is the READ of size 8 in `us_internal_handle_low_priority_sockets`. - `group->low_prio_count` is incremented a second time for a socket that was already counted. It never returns to zero, which is also what `us_socket_group_deinit`'s `low_prio_count == 0` assertion catches in debug/ASan builds. ### Fix In the parking branch, if `low_prio_state == 1` the socket is already in `loop->data.low_prio_head` and not in `group->head_sockets`. Re-disable READABLE (done just above) and leave it where it is instead of group-unlinking and re-counting it. `us_connecting_socket_close` also calls `us_internal_socket_group_unlink_socket` without checking `low_prio_state`, but it only runs before any candidate leg has opened, when every socket in `connecting_head` is still a `SEMI_SOCKET` and cannot have been parked, so it is not affected. ### Test `test/js/bun/net/socket-syscall-fault.test.ts` drives the exact sequence with the in-tree socket fault injection: a `Bun.listen({tls})` server whose every `send` returns 0, and bursts of raw TLS 1.2 clients from a child process. Without the fix the fixture aborts: ``` bun-debug: packages/bun-usockets/src/context.c:68: void us_socket_group_deinit(struct us_socket_group_t *): Assertion `group->low_prio_count == 0' failed. ``` <details> <summary>Verification runs</summary> - Without the fix: 2/2 runs fail with `exitCode: 134`, `signalCode: "SIGABRT"`, and the assertion above. - With the fix: 3/3 runs pass. - `test/js/bun/util/socket-fault-injection.test.ts`, `test/js/node/tls/tls-syscall-fault.test.ts`, `test/js/node/tls/node-tls-server.test.ts`, and `test/js/bun/net/socket.test.ts` produce identical results before and after the change. The two pre-existing environment failures in the last two (plain TCP `ECONNREFUSED` to the just-bound port) reproduce identically on unmodified `main`. </details> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
Jun 29, 2026
…ed (oven-sh#33016) A backend message that fails the connection can share a TCP read with messages that follow it. `PostgresRequest::on_data`'s message loop had no bail-out once `fail()` had run, so the trailing messages in that read kept being dispatched against the already-failed connection. ### Repro A mock backend that answers the StartupMessage with one write carrying two messages: ``` R int32(8) int32(99) Authentication, unrecognized type Z int32(5) 'I' ReadyForQuery ``` ```ts const sql = new SQL({ url: `postgres://u@127.0.0.1:${port}/db`, max: 1, idleTimeout: 1, connectionTimeout: 5 }); await sql`select 1`.catch(() => {}); await Bun.sleep(1600); ``` ### Cause The unrecognized `Authentication` type calls `fail()`, which sets the status to `Failed`, closes the socket, and rejects the pending requests, but the message loop keeps going and dispatches the `ReadyForQuery` from the same read. That calls `set_status(Status::Connected)`, which has no guard against leaving `Failed`, so the dead connection is flipped back to `Connected` and the `on_data` epilogue re-arms its idle timer. uSockets frees a closed `us_socket_t` at the end of the event-loop iteration, so when the timer later fires, `ref_and_close` reads the freed socket: ``` ERROR: AddressSanitizer: heap-use-after-free READ of size 1 at 0x71f2125605d2 thread T0 #0 us_socket_is_closed packages/bun-usockets/src/socket.c:143:21 #4 PostgresSQLConnection::ref_and_close src/sql_jsc/postgres/PostgresSQLConnection.rs:1528:31 #5 PostgresSQLConnection::fail_with_js_value src/sql_jsc/postgres/PostgresSQLConnection.rs:726:14 #6 PostgresSQLConnection::fail_fmt src/sql_jsc/postgres/PostgresSQLConnection.rs:749:14 #7 PostgresSQLConnection::on_connection_timeout src/sql_jsc/postgres/PostgresSQLConnection.rs:557:14 #8 __bun_fire_timer src/runtime/dispatch.rs:1020:35 0x71f2125605d2 is located 18 bytes inside of 104-byte region freed by thread T0 here: #2 us_internal_free_closed_sockets packages/bun-usockets/src/loop.c:305:9 ``` ### Fix - `PostgresRequest::on_data`: the message loop returns once the connection's status is `Failed`. `fail()` is terminal; nothing after it in the same read should be handled (a `DataRow`, `CommandComplete`, or `ErrorResponse` in that position would be just as wrong as the `ReadyForQuery`). - `PostgresSQLConnection::set_status`: refuses to transition out of `Failed`. The transition function owns that invariant; every other consumer of `Status` (the timer interval, `update_has_pending_activity`, the idempotency check in `fail_with_js_value`) already assumes `Failed` is terminal. ### Verification `test/js/sql/postgres-failed-connection-resurrection.test.ts` runs a fixture against the mock backend above and lets it outlive the idle-timer window. Without the fix the fixture dies with the ASan report above; with it the fixture exits 0. Gated to ASan builds because the bug is a read of freed memory, which release lanes do not detect. The postgres fault-injection and integration suites still pass locally (90 tests across `test/js/sql/postgres-*.test.ts`, `sql*.test.ts`, `tls-sql.test.ts`). ### Related - oven-sh#32861 detaches the stored socket handle in `on_close` / `on_connect_error` so nothing can dereference the freed `us_socket_t` regardless of how the stale read is reached. It removes the last step of this chain from the other end; this PR stops the failed connection from being resurrected at all. - oven-sh#30950 guards the JS pool's `handleConnected` against the reverse ordering within one read (a legitimately queued `onconnect` microtask arriving after a synchronous `onclose`).
springmin
pushed a commit
that referenced
this pull request
Jun 30, 2026
…used (oven-sh#33118) ### What does this PR do? Fixes a silent failure in `Bun.serve`: when the `fetch` handler returns a `Response` whose body has already been used (most commonly the same `Response` object returned for more than one request), the request is answered with a `200` and `Content-Length: 0`. Nothing is thrown, nothing is logged, and `error()` is never invoked. ```js const cached = new Response("cached-route-body"); Bun.serve({ fetch() { return cached; } }); // request #1: 200, body "cached-route-body" // request #2+: 200, Content-Length: 0, empty body, error() never fires ``` Per the fetch model a disturbed body is an error, and Deno and workerd both reject it loudly. Bun's static routes (`routes: { "/x": new Response(...) }`) already throw "Response body has already been used" for a used body at registration time; the dynamic path was the only place a disturbed body got silently dropped. Cause: `RequestContext::do_render_with_body` has explicit arms for `Error` bodies, in-memory bodies, and `Locked` streams, but `Body::Value::Used` fell into the catch-all `_ => {}` arm, which renders the never-assigned context blob: a 200 with `Content-Length: 0`. This affected string, `Uint8Array`, stream, and file bodied Responses alike, and also a Response whose body the handler consumed (`await response.text()`) before returning it. Fix: add a `Body::Value::Used` arm that builds a `TypeError` (`code: "ERR_BODY_ALREADY_USED"`, message `Response body already used. A Response body can only be sent once; create a new Response for each request.`) and routes it through `run_error_handler`, exactly like the existing body-error and locked-stream arms. With an `error()` handler, the handler receives the TypeError and its Response is sent. Without one, the error is logged and the request gets the standard 500, the same as any other error thrown from `fetch`. The existing `has_called_error_handler` guard keeps an `error()` handler that itself returns a used Response from recursing. Static routes, fresh Responses per request, bodiless Responses, and HEAD rendering are unchanged; only the `Used` (disturbed) body state is affected. ### How did you verify your code works? `test/js/bun/http/serve-reused-response.test.ts`: - the same string, `Uint8Array`, and stream bodied Response returned twice: the first request gets the body, later ones invoke `error()` with the TypeError (`code: "ERR_BODY_ALREADY_USED"`) and receive its response - a Response consumed with `.text()` before being returned from an async handler: same error - no `error()` handler (`development: false`): the client gets 500 `"Something went wrong!"`, the error is printed to stderr, and the process exit code matches other unhandled fetch errors - fresh Responses and a static-route Response reused across requests keep working and never call `error()` The already-used cases fail on current `bun test` (they observe the empty 200s and zero `error()` calls) and pass with this change; the two no-reuse tests pass both ways by design.
springmin
pushed a commit
that referenced
this pull request
Jul 2, 2026
…h#33186) ### Repro ```sh printf '{"name":"x","version":"1.0.0"}' > package.json bun pm pkg set 'contributors[0]=alice' ``` On a release build (1.4.0 and current `main`) this exits 0 and writes freed heap bytes into `package.json` as the property key: ```json { "name": "x", "version": "1.0.0", "P\x01\x00\x00\x00tors": { "\x00": "alice" } } ``` Depending on what was in the freed allocation the result is often not valid JSON at all. Any `bun pm pkg set` key path containing `[index]` hits it. Under ASAN it is a deterministic `heap-use-after-free`: ``` ERROR: AddressSanitizer: heap-use-after-free READ of size 1 #0 bun_js_printer::write_pre_quoted_string_inner src/js_printer/lib.rs:1014 #7 PmPkgCommand::save_package_json src/runtime/cli/pm_pkg_command.rs:909 freed by thread T0 here: #7 <Box<[u8]> as Drop>::drop #12 PmPkgCommand::set_value src/runtime/cli/pm_pkg_command.rs:661 previously allocated by thread T0 here: #10 <Box<[u8]> as From<&[u8]>>::from #11 PmPkgCommand::parse_key_path src/runtime/cli/pm_pkg_command.rs:583 ``` <details> <summary>full ASAN report</summary> ``` ================================================================= ==16563==ERROR: AddressSanitizer: heap-use-after-free on address 0x73423c7c0670 at pc 0x00000f583cc5 bp 0x7fff2667e950 sp 0x7fff2667e948 READ of size 1 at 0x73423c7c0670 thread T0 #0 0x00000f583cc4 in _RINvCs59Hqei94dXF_14bun_js_printer29write_pre_quoted_string_innerINtB2_16StdWriterAdapterQINtB2_6WriterNtB2_12BufferWriterEEKVNtNtB2_8Encoding4Utf8UECsgBGN0jRPILJ_11bun_bundler /workspace/bun/src/js_printer/lib.rs:1014:79 #1 0x00000ebda439 in <bun_js_printer::__gated_printer::Printer<&mut bun_js_printer::Writer<bun_js_printer::BufferWriter>, false, false, false, true, false>>::print_string_characters_utf8 /workspace/bun/src/js_printer/lib.rs:2641:21 #2 0x00000ebdb7a7 in <bun_js_printer::__gated_printer::Printer<&mut bun_js_printer::Writer<bun_js_printer::BufferWriter>, false, false, false, true, false>>::print_string_characters_e_string /workspace/bun/src/js_printer/lib.rs:4546:22 #3 0x00000ebdb238 in <bun_js_printer::__gated_printer::Printer<&mut bun_js_printer::Writer<bun_js_printer::BufferWriter>, false, false, false, true, false>>::print_string_literal_e_string /workspace/bun/src/js_printer/lib.rs:3018:18 #4 0x00000ebd2be2 in <bun_js_printer::__gated_printer::Printer<&mut bun_js_printer::Writer<bun_js_printer::BufferWriter>, false, false, false, true, false>>::print_property /workspace/bun/src/js_printer/lib.rs:4807:34 #5 0x00000ebc0166 in <bun_js_printer::__gated_printer::Printer<&mut bun_js_printer::Writer<bun_js_printer::BufferWriter>, false, false, false, true, false>>::print_expr /workspace/bun/src/js_printer/lib.rs:3962:38 #6 0x00000ee574c1 in bun_js_printer::print_json::<&mut bun_js_printer::Writer<bun_js_printer::BufferWriter>> /workspace/bun/src/js_printer/lib.rs:8071:13 #7 0x00000c05c270 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::save_package_json /workspace/bun/src/runtime/cli/pm_pkg_command.rs:909:25 #8 0x00000c060cfb in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::exec_set /workspace/bun/src/runtime/cli/pm_pkg_command.rs:330:13 #9 0x00000c05d333 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::exec /workspace/bun/src/runtime/cli/pm_pkg_command.rs:73:32 #10 0x00000bf919d0 in <bun_runtime::cli::package_manager_command::PackageManagerCommand>::exec /workspace/bun/src/runtime/cli/package_manager_command.rs:704:13 #11 0x00000c3fbb87 in bun_runtime::cli::command::exec_pm /workspace/bun/src/runtime/cli/mod.rs:1591:34 #12 0x00000c3f2b86 in bun_runtime::cli::command::start /workspace/bun/src/runtime/cli/mod.rs:1309:43 #13 0x00000bfad16c in bun_runtime::cli::cli::start /workspace/bun/src/runtime/cli/mod.rs:573:27 #14 0x00000bb3c034 in main /workspace/bun/src/bun_bin/lib.rs:230:5 #15 0x77223ccc7ca7 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 oven-sh#16 0x77223ccc7d64 in __libc_start_main csu/../csu/libc-start.c:360:3 oven-sh#17 0x0000099d1d1d in __wrap___libc_start_main /workspace/bun/build/debug/../../src/jsc/bindings/workaround-missing-symbols.cpp:487:12 0x73423c7c0670 is located 0 bytes inside of 12-byte region [0x73423c7c0670,0x73423c7c067c) freed by thread T0 here: #0 0x000007ae192a in free crtstuff.c #1 0x00000bb3c5a7 in <std::alloc::System as core::alloc::global::GlobalAlloc>::dealloc /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/alloc/unix.rs:48:18 #2 0x00000bb3be9a in __rustc::__rust_dealloc /workspace/bun/src/bun_bin/lib.rs:56:15 #3 0x00001258b05f in alloc::alloc::dealloc_nonnull /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:128:14 #4 0x0000125872fe in <alloc::alloc::Global>::deallocate_impl_runtime /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:229:22 #5 0x000012586364 in <alloc::alloc::Global>::deallocate_impl /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:344:9 #6 0x00001258d79c in <alloc::alloc::Global as core::alloc::Allocator>::deallocate /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:462:23 #7 0x000012582946 in <alloc::boxed::Box<[u8]> as core::ops::drop::Drop>::drop /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1956:24 #8 0x000012572e44 in core::ptr::drop_in_place::<alloc::boxed::Box<[u8]>> /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:809:1 #9 0x000011f8d429 in core::ptr::drop_in_place::<[alloc::boxed::Box<[u8]>]> /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:809:1 #10 0x00000ef6b73a in <alloc::vec::Vec<alloc::boxed::Box<[u8]>> as core::ops::drop::Drop>::drop /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/vec/mod.rs:4258:13 #11 0x00000ef69e64 in core::ptr::drop_in_place::<alloc::vec::Vec<alloc::boxed::Box<[u8]>>> /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:809:1 #12 0x00000c061846 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::set_value /workspace/bun/src/runtime/cli/pm_pkg_command.rs:661:5 #13 0x00000c061038 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::exec_set /workspace/bun/src/runtime/cli/pm_pkg_command.rs:325:13 #14 0x00000c05d333 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::exec /workspace/bun/src/runtime/cli/pm_pkg_command.rs:73:32 #15 0x00000bf919d0 in <bun_runtime::cli::package_manager_command::PackageManagerCommand>::exec /workspace/bun/src/runtime/cli/package_manager_command.rs:704:13 oven-sh#16 0x00000c3fbb87 in bun_runtime::cli::command::exec_pm /workspace/bun/src/runtime/cli/mod.rs:1591:34 oven-sh#17 0x00000c3f2b86 in bun_runtime::cli::command::start /workspace/bun/src/runtime/cli/mod.rs:1309:43 oven-sh#18 0x00000bfad16c in bun_runtime::cli::cli::start /workspace/bun/src/runtime/cli/mod.rs:573:27 oven-sh#19 0x00000bb3c034 in main /workspace/bun/src/bun_bin/lib.rs:230:5 oven-sh#20 0x77223ccc7ca7 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 previously allocated by thread T0 here: #0 0x000007ae1bc8 in malloc crtstuff.c #1 0x00000bb3c520 in <std::alloc::System as core::alloc::global::GlobalAlloc>::alloc /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/alloc/unix.rs:14:22 #2 0x00000bb3be30 in __rustc::__rust_alloc /workspace/bun/src/bun_bin/lib.rs:56:15 #3 0x00001258b335 in alloc::alloc::alloc /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:101:9 #4 0x000012586b81 in <alloc::alloc::Global>::alloc_impl_runtime /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:210:73 #5 0x0000125862b6 in <alloc::alloc::Global>::alloc_impl /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:332:9 #6 0x00001258d86a in <alloc::alloc::Global as core::alloc::Allocator>::allocate /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:449:14 #7 0x00001257dbd3 in <alloc::boxed::Box<[u8]>>::try_clone_from_ref_in /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:881:29 #8 0x00001257da49 in <alloc::boxed::Box<[u8]>>::clone_from_ref_in /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:840:15 #9 0x00001257d3f4 in <alloc::boxed::Box<[u8]>>::clone_from_ref /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:793:9 #10 0x000012581e34 in <alloc::boxed::Box<[u8]> as core::convert::From<&[u8]>>::from /root/.rustup/toolchains/nightly-2026-05-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed/convert.rs:77:9 #11 0x00000c05a47f in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::parse_key_path /workspace/bun/src/runtime/cli/pm_pkg_command.rs:583:37 #12 0x00000c061608 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::set_value /workspace/bun/src/runtime/cli/pm_pkg_command.rs:643:30 #13 0x00000c061038 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::exec_set /workspace/bun/src/runtime/cli/pm_pkg_command.rs:325:13 #14 0x00000c05d333 in <bun_runtime::cli::pm_pkg_command::PmPkgCommand>::exec /workspace/bun/src/runtime/cli/pm_pkg_command.rs:73:32 ``` </details> ### Cause `parse_key_path` returned a `Vec<Box<[u8]>>`, and `set_value` / `set_nested` inserted those boxed segments into the manifest AST by reference: `E::Object::put` constructs `EString::init(key)`, whose documented contract is that `key` is arena-owned (it records the slice, it does not copy it). The vector is a local of `set_value`, so it dropped before `exec_set` reached `save_package_json`, and the JSON printer then read the dangling keys. The non-bracket path in `set_value` did not have the bug: it borrowed its segments straight out of the argv key, which outlives the whole command. The bracket path differed only by the unnecessary boxing. ### Fix `parse_key_path` now returns `Vec<&[u8]>`. Every segment is a literal sub-slice of the input key, so nothing ever needed owning. With the boxing gone, `set_value`'s separate non-bracket branch and its `set_nested_simple` helper (which existed only to avoid the allocation) were exact duplicates of the bracket path, so they are deleted and all keys route through `parse_key_path` + `set_nested`. `set_nested_simple`'s trailing `root.put(current_key, nested)` was a no-op: `ExprData::EObject` is a `StoreRef` handle, so mutating the copy returned by `root.get()` already mutates the stored object, and the put re-stores the same handle. Dropping it with the function changes nothing (and the prior bracket path, `set_nested`, never had it). Intentionally not changed here: `set 'contributors[0]=alice'` produces `"contributors": {"0": "alice"}`, an object keyed by the digit string, rather than the array npm's `pkg set` creates, and `set 'array[]=x'` still errors with `InvalidPath` instead of appending. Both are the npm compat gap tracked in oven-sh#22035, which is separate from the memory safety of the key names and is not closed by this PR. ### Verification New test in `test/cli/install/bun-pm-pkg.test.ts` reparses the written file and asserts the exact object. Without the fix it fails on release (`SyntaxError: JSON Parse error: Invalid escape character x`) and on the ASAN debug build (the child aborts on the use-after-free). With the fix the full `bun-pm-pkg.test.ts` suite passes (74 pass, 0 fail).
springmin
pushed a commit
that referenced
this pull request
Jul 2, 2026
…en-sh#33242) ### What After a 3xx redirect, `handle_response_metadata` rewrites per-hop request state on the HTTP-thread clone of the `AsyncHTTP`: - `client.url` (and `connected_url`) become a self-borrow into `client.redirect`, a `Vec<u8>` the clone owns and frees in the final-callback teardown (`AsyncHTTP::on_async_http_callback_raw`). - On a cross-origin hop, `Authorization`/`Proxy-Authorization`/`Cookie`/`Host` are removed from `client.header_entries` in place. - The method may be downgraded to GET. `NetworkTask::notify`'s bitwise copy-back (`ptr::write(real, ptr::read(async_http))`) carries all of that into the JS-thread `AsyncHTTP`. When `bun install` retries the task after a retryable failure (5xx or a connection reset on the redirect target), the re-scheduled request therefore: 1. connects through the freed redirect buffer (use after free), and 2. if the redirect was cross-origin, goes out without `Authorization`, so an authorized registry answers 401. ASAN (debug build), deterministic on the first try: ``` ERROR: AddressSanitizer: heap-use-after-free ... thread T1 (HTTP Client) READ of size 1 #0 bun_core::fmt::parse_int::<u16> src/bun_core/fmt.rs:929 #1 <bun_url::URL>::get_port src/url/lib.rs:470 #2 <bun_url::URL>::get_port_auto src/url/lib.rs:474 #3 <bun_http::http_thread::HttpThread>::connect src/http/HTTPThread.rs:602 #4 <bun_http::HTTPClient>::start_ src/http/lib.rs:2635 #6 <bun_http::async_http::AsyncHTTP>::on_start src/http/AsyncHTTP.rs:893 freed by thread T1 (HTTP Client): <bun_http::async_http::AsyncHTTP>::on_async_http_callback_raw src/http/AsyncHTTP.rs:774 previously allocated by thread T1 (HTTP Client): <bun_http::HTTPClient>::handle_response_metadata src/http/lib.rs:5038 ``` On a release build the same sequence does not crash, but the retries never reach the server (each one connects through freed memory) and the install fails. ### Repro A scripted registry where the manifest URL 302-redirects and the redirect target answers a 500 once, then the real packument: ``` GET /BaR -> 302 Location: /redirected/BaR GET /redirected/BaR -> 500 on the first hit, then the packument GET /BaR-0.0.2.tgz -> tarball ``` `bun install` against it aborts under ASAN and fails on release. Any 301/302/307/308 and 1- or 2-hop chains hit the same path. With an authorized registry that redirects cross-origin (the common Artifactory / CodeArtifact / GitHub Packages shape), the retry also loses `Authorization`; that variant fails with `GET <registry>/BaR - 401` even once the URL is fixed. ### Fix `src/http/AsyncHTTP.rs`: the `!has_more` teardown block already releases every clone-owned allocation. Before freeing `client.redirect`, restore the per-hop state that a re-scheduled attempt must not inherit: - `client.url` back to the caller-owned pre-redirect URL (`AsyncHTTP.url`, which borrows memory valid for the original's whole lifetime), and `client.connected_url` (which `connect` derives from it) to default. - `client.header_entries` back to the untouched `AsyncHTTP.request_headers`. The list is bitwise-shared with the JS-thread original, so it must not be dropped or reallocated on the HTTP thread; it was cloned from `request_headers` at init and only ever shrinks, so `clear_retaining_capacity()` + `append_list_assume_capacity()` restores it in place. - `client.method` back to `AsyncHTTP.method`. Nothing that crosses back to the JS thread references clone-freed memory anymore, and a retried request restarts from the original URL with the original headers instead of the last redirect hop's, which is what the install-level retry is meant to do. ### Tests `test/cli/install/bun-install-retry.test.ts`: - `retries a manifest whose redirect target 500s once` - `retries a tarball whose redirect target 500s once` (the sibling retry site in `runTasks`) - `retries an authorized manifest whose cross-origin redirect target 500s once` (also asserts the cross-origin hop itself still does NOT carry `Authorization`, so the spec-mandated strip is unchanged) All three fail on the unfixed build (ASAN abort under `bun bd`, install error with `USE_SYSTEM_BUN=1`). The third additionally fails with a 401 if only the URL is restored and not the headers, so each restore is load-bearing. `test/js/web/fetch/fetch-redirect.test.ts` and `fetch-url-after-redirect.test.ts` still pass, so `response.url` after a redirect is unaffected (it comes from the owned `metadata.url` copy, not from `client.url`). --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
Jul 4, 2026
…ipeReader::on_read_chunk fails (oven-sh#33269) ## Problem Heap use-after-free in Bun Shell when `epoll_ctl(MOD)` fails while the shell `PipeReader::on_read_chunk` callback re-registers the poll from inside the read loop. Found by syscall-fault-injection fuzzing against `origin/main`. This is the path oven-sh#32986 called out as out of scope: that PR fixed `read_with_fn`'s own `EAGAIN`-arm re-registration, but the shell `PipeReader::on_read_chunk` still called `self.reader.register_poll()` itself. ``` ==ERROR: AddressSanitizer: heap-use-after-free READ of size 8 #0 <PosixBufferedReader>::read_with_fn src/io/PipeReader.rs:837:43 #1 <PosixBufferedReader>::read_socket src/io/PipeReader.rs:581:9 #2 <PosixBufferedReader>::on_poll src/io/PipeReader.rs:534:17 #3 __bun_run_file_poll src/runtime/dispatch.rs:677:22 ``` <details> <summary>Freed-by stack (the re-entrant callback chain)</summary> ``` freed by thread T0 here: core::ptr::drop_in_place::<Arc<shell::subproc::PipeReader>> <shell::subproc::PipeReader>::on_reader_error src/runtime/shell/subproc.rs:2363 <PosixBufferedReader>::register_poll src/io/PipeReader.rs:433 <shell::subproc::PipeReader>::on_read_chunk src/runtime/shell/subproc.rs:2062 <PosixBufferedReader>::read_with_fn src/io/PipeReader.rs:875 <PosixBufferedReader>::read_socket <PosixBufferedReader>::on_poll __bun_run_file_poll ``` </details> ## Repro 1. A shell pipe's `FilePoll` fires and `__bun_run_file_poll` dispatches into `PosixBufferedReader::on_poll` -> `read_with_fn` with a bare `&mut` and no keepalive. 2. `recv()` drains more than half of the 256 KB scratch buffer in one call, so `read_with_fn`'s streaming inner loop flushes the head mid-loop: `parent.vtable.on_read_chunk(.., Progress)`. 3. Shell `PipeReader::on_read_chunk` re-arms the poll itself: `self.reader.register_poll()`. The `epoll_ctl(MOD)` fails (`ENOMEM` in the repro; fd/watch pressure in the wild). 4. `register_poll` dispatches `on_reader_error`. The shell `PipeReader::on_reader_error` signals the `Cmd` and drops the `Readable::Pipe` `Arc`; its own `guard_from_raw` keepalive becomes the last reference, and dropping it frees the `PipeReader` (and the `PosixBufferedReader` embedded in it). 5. `register_poll` returns `false`, but `on_read_chunk` is not a direct caller of the read loop, so the `false` never reaches it. The inner loop keeps going and reads `parent._offset` from the freed reader on the next `recv`. ## Cause `BufferedReaderParent`'s contract (and the `SAFETY` comments in `read_with_fn` / `read_blocking_pipe`) is that `on_read_chunk` never frees the reader; only `on_reader_error` may. The shell `PipeReader::on_read_chunk` broke that transitively by calling `register_poll()`, whose failure path dispatches `on_reader_error`. oven-sh#32986's `register_poll() -> bool` return value only protects direct callers in the read loop. It cannot protect a caller that reaches `register_poll` through the `on_read_chunk` vtable dispatch two frames down. ## Fix Delete the re-arm from shell `PipeReader::on_read_chunk`. It was redundant on both platforms and the codebase already documents why: - POSIX: every exit of `read_with_fn` / `read_blocking_pipe` that wants more data already calls `register_poll()` itself, driven by the `bool` `on_read_chunk` returns. - Windows: `WindowsBufferedReader::on_read` notes "the re-arm is already handled by `on_file_read`'s epilogue / `uv_read_start`", and it already performs the `_buffer.clear()` that used to be `start_with_current_pipe()`'s second side effect. - The sibling shell reader, `IOReader::on_read_chunk_cb`, already dropped its identical re-arm for the same two reasons (redundancy, plus re-deriving `&mut` to the embedded reader while the read loop holds one). Removing it also removes the only `&mut self.reader` re-derivation inside the callback, and the `Output::panic("TODO: ...")` that was the Windows branch's only error handling. ## Test New `SHELL_RECV_BULK=N` mode in `test/js/bun/shell/shell-pipe-read-fault.test.ts`'s `LD_PRELOAD` shim: the first N real `recv()`s on each `AF_UNIX` socket instead return the caller's whole buffer filled with `'A'`. Combined with the existing `SHELL_RECV_EAGAIN_FIRST=1` and `SHELL_FAIL_EPOLL_FROM=3`, one fabricated bulk recv deterministically pushes `head_start` past the half-buffer cutoff so the mid-loop flush (and therefore the failing re-registration) happens from `on_read_chunk`. With the epoll failure count unchanged, the same `epoll_ctl` #3 that used to be issued by `on_read_chunk` is now the read loop's own `EAGAIN` re-registration, whose failure path already returns without touching the reader, so the command just reports `ENOMEM`. - Before the fix: the new test fails in ~750 ms with the `heap-use-after-free` above; the other 6 tests in the file pass. - After the fix: all 7 pass. The test is `skipIf(!isASAN)` like its sibling. Also ran the rest of `test/js/bun/shell/` (`bunshell*.test.ts`: 394 pass / 0 fail; `commands/` and the remaining files: every failure reproduces identically with `src/runtime/shell/subproc.rs` reverted to `main`, so they are pre-existing in this environment, not caused by this change).
springmin
pushed a commit
that referenced
this pull request
Jul 4, 2026
…buffer cannot be allocated (oven-sh#33326) Fixes a `Segmentation fault at address 0x00000040` (sometimes `0x00000030`) reported from Windows x64 builds, crashing inside boringssl's record copy from uSockets' TLS read loop: ``` memcpy src/vctools/crt/vcruntime/src/string/amd64/memcpy.asm bssl::OPENSSL_memcpy vendor/boringssl/crypto/internal.h:868 SSL_peek vendor/boringssl/ssl/ssl_lib.cc:947 SSL_read vendor/boringssl/ssl/ssl_lib.cc:918 us_internal_ssl_on_data packages/bun-usockets/src/crypto/openssl.c:1797 us_internal_dispatch_ready_poll packages/bun-usockets/src/loop.c:600 uv__fast_poll_process_poll_req vendor/libuv/src/win/poll.c:208 uv_run vendor/libuv/src/win/core.c:737 ``` ## Cause `us_internal_init_loop_ssl_data` (`openssl.c:677`) allocates one 512 KiB plaintext buffer per event loop, lazily, on the loop's first TLS socket, and never checked the result: ```c loop_ssl_data->ssl_read_output = us_malloc(LIBUS_RECV_BUFFER_LENGTH + LIBUS_RECV_BUFFER_PADDING * 2); ``` With `ssl_read_output == NULL`, every later `SSL_read` hands boringssl ```c loop_ssl_data->ssl_read_output + LIBUS_RECV_BUFFER_PADDING + read ``` as its plaintext destination, so the first record of application data memcpy's to `NULL + 32`. `SSL_peek`'s `OPENSSL_memcpy(buf, ...)` at `ssl_lib.cc:947` is the write, and the access violation confirms it is a write fault. `0x30`/`0x40` rather than `0x20` is memcpy's destination-alignment preamble (`dst += VEC_SIZE; dst &= ~(VEC_SIZE - 1)`), which moves the first faulting store for copies larger than eight vector registers. Measured on the copy sizes a real TLS record produces: | memcpy variant | first faulting store for `dst = NULL + 32` | | --- | --- | | 32-byte vectors (AVX) | `0x40` | | 16-byte vectors (SSE) | `0x30` | So the two strikingly stable fault addresses are just CPU dispatch across the affected machines, and `read` is always `0`: the crash is always the connection's first record of application data. Only Windows reports it because Linux and macOS overcommit, so a 512 KiB `malloc` there effectively never returns NULL. Windows fails the commit cleanly, and the loop's much smaller `us_calloc` still succeeds out of an already-committed page, leaving exactly the observed shape: a valid `loop_ssl_data` whose `ssl_read_output` is NULL. ## Fix - Null-check the buffer allocation, the `us_calloc` of `loop_ssl_data`, and the `BIO_meth_new`/`BIO_new` calls beside them, and route the failure through Bun's out-of-memory crash path (`Bun__outOfMemory`, new C entry point next to `Bun__panic`). The process now dies with `Bun ran out of memory` and a stack trace that names the allocation, instead of faulting on the first TLS byte. - Apply the same check to the sibling site: `recv_buf`/`send_buf` in `us_internal_loop_data_init` are the same unchecked `malloc(LIBUS_RECV_BUFFER_LENGTH + LIBUS_RECV_BUFFER_PADDING * 2)`. A NULL `recv_buf` does not fault, it makes every read on the loop fail with `EFAULT` for the life of the process, which is worse to diagnose. - `us_internal_free_loop_ssl_data` left `loop->data.ssl_data` dangling, which defeats the `if (!loop->data.ssl_data)` guard the init function relies on. It now clears the field. A 512 KiB `malloc` effectively never returns NULL on an overcommitting kernel, so the failure path needs the existing socket fault injector to be reachable from a test. This adds an `ssl_loop_buffer` rule to it, which like the rest of the injector is compiled out of release builds. ## Verification The new test spawns a child that arms `ssl_loop_buffer` before its first TLS socket and asserts it reports out of memory rather than reaching a read loop. Reverting only `if (!loop_ssl_data->ssl_read_output) Bun__outOfMemory();` reproduces the reported crash exactly, on Linux, from that same fixture: same fault address, same frames, same boringssl source lines. <details> <summary>Reproduction on the unfixed build (<code>bun bd</code>, ASAN)</summary> ``` ==19592==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000040 ==19592==The signal is caused by a WRITE memory access. ==19592==Hint: address points to the zero page. #0 __memcpy_evex_unaligned_erms #1 bssl::OPENSSL_memcpy(void*, void const*, unsigned long) vendor/boringssl/crypto/internal.h:868:10 #2 SSL_peek vendor/boringssl/ssl/ssl_lib.cc:947:3 #3 SSL_read vendor/boringssl/ssl/ssl_lib.cc:918:13 #4 us_internal_ssl_on_data packages/bun-usockets/src/crypto/openssl.c:1847:21 #5 us_internal_dispatch_ready_poll packages/bun-usockets/src/loop.c:625:38 ``` With the fix: ``` panic(main thread): Bun ran out of memory Bun__outOfMemory src/bun_bin/phase_c_exports.rs:81:5 us_internal_init_loop_ssl_data packages/bun-usockets/src/crypto/openssl.c:696:42 us_internal_ssl_attach packages/bun-usockets/src/crypto/openssl.c:1275:3 ``` </details> `test/js/node/tls/tls-syscall-fault.test.ts` (11 pass), `test/js/bun/util/socket-fault-injection.test.ts` (15 pass), plus `socket-syscall-fault`, `serve-syscall-fault` and `fetch-syscall-fault` (19 pass) are green. The three failures in `test/js/node/tls/` on this machine are pre-existing: two also fail on an unmodified 1.4.0, and `tls.connect should ignore invalid NODE_EXTRA_CA_CERTS` takes 5.75s, just over the 5s local default (CI triples the per-test timeout for ASAN builds). ## Teardown audit The report also asked whether a socket can reach `us_internal_ssl_on_data` after its loop's SSL data has been freed. `us_internal_free_loop_ssl_data` is only reachable from `us_loop_free`, and the only loop Bun frees today is `SpawnSyncEventLoop`'s, which never has a TLS socket attached (its `loop->data.ssl_data` is always NULL, so the free is a no-op). So that is not the cause here. It is worth noting separately that the libuv `us_loop_free` (`eventing/libuv.c:201-206`) calls `us_internal_loop_data_free(loop)` and *then* runs `uv_run(loop->uv_loop, UV_RUN_NOWAIT)`, which is a full libuv iteration that can dispatch socket poll callbacks into the just-freed `recv_buf` and `ssl_data`. The POSIX `us_loop_free` (`eventing/epoll_kqueue.c:56-60`) has no such window. It is unreachable today for the reason above, so it is left out of this PR rather than changing loop teardown ordering without a test that can exercise it.
springmin
pushed a commit
that referenced
this pull request
Jul 10, 2026
…lementation (oven-sh#33849) ### What Five self-contained fixes (one commit each) for bugs in the C++ WebStreams implementation introduced by oven-sh#33193, found while auditing the rewrite. Two are memory-safety issues reachable from a few lines of user JS in release builds; three are hangs/data loss via reentrancy. Each commit ships regression tests verified to fail before the fix and pass after. --- ### 1. Foreign-realm `newTarget` type-confuses the global object (all 10 constructors) Every stream constructor's subclass slow path did `uncheckedDowncast<JSDOMGlobalObject>(newTargetGlobalObject)`. A `node:vm` context's global is a *sibling* class, so the downcast is an invalid `static_cast` in release builds — `getDOMStructure` then reads and writes structure caches past the end of the smaller allocation. ```js const vm = require("node:vm"); const foreignFn = vm.runInContext("(function F(){})", vm.createContext({})); Reflect.construct(ReadableStream, [], foreignFn); // asserts in debug; heap type confusion in release ``` The ten copy-pasted `structureForNewTarget` statics are replaced with one shared template in `StreamConstructor.h` that `dynamicDowncast`s and falls back to the constructor's own realm's cached Structure (per-VM correct, unlike a process-global fallback). ### 2. `TransferArrayBuffer` left transferred buffers resizable The spec defines TransferArrayBuffer as `ArrayBufferCopyAndDetach(O, undefined, fixed-length)`, but `transferArrayBufferImpl` used `ArrayBuffer::transferTo`, which carries `maxByteLength` across the transfer. User JS reaching the stream-internal buffer through `byobRequest.view.buffer` could `resize()` it, invalidating every byte length the controller recorded: ```js const rs = new ReadableStream({ type: "bytes", pull(c) { c.byobRequest.view.buffer.resize(0); // succeeded; must throw TypeError c.enqueue(new Uint8Array(10)); // RELEASE_ASSERT → process abort, release builds included }, }); await rs.getReader({ mode: "byob" }).read( new Uint8Array(new ArrayBuffer(64, { maxByteLength: 1024 })), ); ``` A second variant (`resize(2)` + `respond()` with a remainder) made the remainder-clone path `subspan` past the live length — an out-of-bounds heap read whose bytes were delivered to a subsequent `read()`. Fix mirrors JSC's own `arrayBufferCopyAndDetach` FixedLength slow path: resizable sources are copied into a fixed-length block, then detached; non-resizable buffers keep the zero-copy transfer. (The WPT streams suite has no resizable-ArrayBuffer coverage, so tests are added.) ### 3. Bulk drain ran the user `pull()` before `ResetQueue` `drainQueueEntriesInto` — behind `reader.readMany()` and the buffered consumers (`text()`, `bytes()`, `Bun.readableStreamTo*`) — removed every queue entry, ran the close/pull step, and only then reset the queue. A chunk enqueued synchronously by that pull landed in the still-live queue and was wiped by the reset; a `close()` in the same pull saw a momentarily non-empty queue and never re-evaluated: ```js let pulls = 0; const rs = new ReadableStream({ start(c) { c.enqueue("A"); }, pull(c) { if (++pulls >= 2) { c.enqueue("B"); c.close(); } }, }, { highWaterMark: 2 }); await Bun.sleep(0); await rs.text(); // hung forever (and "B" was silently destroyed); now resolves "AB" ``` The queue is now reset before the close/pull step. The pull *decision* still runs against the pre-drain `[[queueTotalSize]]`, preserving the existing readMany batching cadence (the `readMany batches the pipelined pull's chunk` test still passes byte-for-byte). ### 4. Async iterator: reentrant `next()`/`return()` from a synchronous `pull()` The iterator published `m_ongoingPromise` only *after* running steps that invoke the user `pull()` synchronously. A `return()` called from inside that pull saw a stale non-pending ongoing promise, skipped the chaining path, and released the reader under an in-flight read (`ASSERT(reader->m_readRequests.isEmpty())` in debug): ```js let it, phase = 0; const rs = new ReadableStream({ pull(c) { if (++phase === 2) { it.return("bye"); return new Promise(() => {}); } }, }, { highWaterMark: 1 }); it = rs.values(); await null; await null; await null; it.next(); // pull #2 fires synchronously and reenters via it.return() → assert/double release ``` The result promise is now published before any user JS can run — but only when the current ongoing promise is not pending, so ongoing-settled reactions never rewind the chain tail (queued `next()` calls still resolve in call order; tests cover both properties). ### 5. `TextEncoderStream`/`TextDecoderStream` let transform failures escape synchronously The codec transform/flush algorithms wrapped only the encode/decode call in the completion-record catch. Both run user JS (`ToString` of the chunk; the patchable `TextDecoder.prototype.decode`), which can cancel the readable mid-transform — the subsequent enqueue's TypeError then escaped synchronously out of `writer.write()`/`writer.close()`, and the in-flight operation never settled: ```js const tes = new TextEncoderStream(); const reader = tes.readable.getReader(); const writer = tes.writable.getWriter(); reader.read(); await null; await null; await null; try { writer.write({ toString() { reader.cancel(); return "x"; } }); // threw synchronously (spec: never throws) } catch {} await writer.abort("bye"); // never settled — wedged forever ``` The catch now covers the enqueue at all sites (encoder transform+flush unified into one helper, mirroring the decoder), converting abrupt completions into rejected promises that flow through `transformStreamError` — write rejects, abort settles, matching Node. --- ### Test notes - All 9 regression tests fail on the unfixed implementation (crash / 5s-timeout hang / sync throw) and pass with the fixes; verified by reverting `src/jsc/bindings/webcore/streams/` to main and re-running. - Full `test/js/web/streams/` + WPT streams + encoding suites: 1,421 tests, no regressions (the one pre-existing failure, `streams-leak` absolute-RSS floor under ASAN, fails identically on main with a negative RSS delta). <!-- robobun:evidence:begin --> --- **no test proof** · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/web/streams/streams.test.js <!-- robobun:evidence:end --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: robobun <117481402+robobun@users.noreply.github.com> Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
springmin
pushed a commit
that referenced
this pull request
Jul 11, 2026
…ven-sh#33931) ## Symptom Intermittent `EBADF: bad file descriptor, fstat` in unrelated `Bun.file(path).text()` calls on Windows. The most visible CI victim is `test/cli/install/bun-install.test.ts` (15 flaky hits across the last 40 PR builds, spread across many different test cases), because its dummy registry serves every tarball via `new Response(Bun.file(path))`. ``` EBADF: bad file descriptor, fstat syscall: "fstat", errno: -9, code: "EBADF" at async <anonymous> (test/cli/install/bun-install.test.ts:6461) ``` ## Cause `FileResponseStream::start` clears `ReaderFlags::CLOSE_HANDLE` on its `BufferedReader` so it can close the fd itself in `Drop` (src/runtime/server/FileResponseStream.rs:182). `PosixBufferedReader` checks that flag before closing (src/io/PipeReader.rs:283/356/374/385). `WindowsBufferedReader` defines the flag (:1143) and sets it in `Default` (:1155) but never reads it, so `close_impl`'s `Source::File` arm unconditionally calls `File::detach()` which queues `uv_fs_close` on the same CRT fd that `FileResponseStream::Drop` already queued a `Closer::close` for (:547). Both closes are async on the libuv threadpool. Between close #1 freeing the CRT slot and close #2 running, an unrelated `uv_fs_open` (from another `Response(Bun.file)` open, or a `Bun.file().text()`) can be handed the recycled slot; close #2 then closes the wrong fd, and its next `fstat` or `read` sees EBADF. ## Fix Honor `WindowsFlags::CLOSE_HANDLE` in `WindowsBufferedReader::close_impl` via a new `File::detach_borrowed_fd()` that mirrors `detach()` but leaves `close_after_operation` unset, so no `uv_fs_close` is scheduled for a parent-owned fd: - `close_impl` with `CLOSE_HANDLE` set: unchanged (`detach()` schedules `uv_fs_close` now or after the pending read). - `close_impl` with `CLOSE_HANDLE` cleared: `detach_borrowed_fd()`. If idle, drop the `Box<File>` there; if a read is in flight, null `fs.data` and let `on_file_read`'s detached branch reclaim the Box after `complete()`. - `on_file_read`'s `parent_ptr.is_null()` path now handles both shapes (state `Closing`: `on_close_complete` frees; otherwise: free here). `BaseWindowsPipeWriter::close`'s `!owns_fd()` branch already open-coded the same sequence and now routes through the shared `detach_borrowed_fd()`, so the reader and writer close paths share one contract. ## Verification The double-close only exists on Windows (POSIX honors the flag), so the Linux gate cannot observe fail-before. Windows x64-baseline at 91675d0: - fail-before: 5/15 runs of the new test fail under the system bun with `EBADF: bad file descriptor, fstat 'served.bin'` - pass-after: 8/8 under the debug build with this patch The new test is gated behind `isWindows`. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 2 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/http/bun-serve-file.test.ts <!-- robobun:evidence:end --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
Jul 13, 2026
…n-sh#34078) ### What does this PR do? `bsd_create_unix_socket_address()` takes the caller's path as `(const char *path, size_t path_len)` and, on Linux, works around `sun_path`'s 108-byte limit by opening the parent directory and binding to `/proc/self/fd/<dirfd>/<basename>` instead. The basename was being copied with ```c snprintf(sun_path, sizeof sun_path, "/proc/self/fd/%d/%s", fd, path + dirname_len); ``` but `path` is a ptr+len pair coming from a Rust `&[u8]` with no NUL terminator. `%s` walks past the end of the allocation. On ASan builds this aborts with `heap-buffer-overflow`; on release builds `sun_path` is assembled from whatever heap bytes follow the path buffer, so the kernel sees an address built from out-of-bounds memory (sometimes the right one, sometimes `EINVAL`, sometimes something else). The trigger window is any pathname unix socket with `108 <= path_len` whose basename still fits inside `/proc/self/fd/N/`, reachable from `net.createServer().listen(path)`, `net.connect(path)`, `Bun.listen({unix})` and `Bun.connect({unix})`. Node binds a full 108-byte `sun_path` here, so this is also a parity break at exactly length 108. Fix: use `%.*s` with `(int)(path_len - dirname_len)` so the copy is bounded by the known basename length. ### Repro ```js import * as net from "node:net"; import * as fs from "node:fs"; const dir = fs.mkdtempSync("/tmp/sun108-"); const path = dir + "/" + "l".repeat(108 - dir.length - 1); // exactly 108 bytes net.createServer().listen(path, () => { console.log("LISTENING"); process.exit(0); }); ``` Before (debug/ASan): ``` ==510==ERROR: AddressSanitizer: heap-buffer-overflow ... READ of size 90 at 0x7339f260062c thread T0 #0 ... in printf_common #2 ... in snprintf #3 ... in bsd_create_unix_socket_address packages/bun-usockets/src/bsd.c ``` After: `LISTENING`, exit 0. ### How did you verify your code works? `bun bd test test/js/bun/net/unix-socket-long-path.test.ts` passes (4/4). With the `packages/` change stashed out, all four cases fail with the ASan `heap-buffer-overflow` header in the subprocess stderr. <!-- robobun:evidence:begin --> --- **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/bun/net/unix-socket-long-path.test.ts <!-- robobun:evidence:end --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
Jul 15, 2026
…ose_slave_fd (oven-sh#34225) ### What does this PR do? Fixes a regression from oven-sh#33882 where a script that spawns a subprocess with an inline `terminal:` option and awaits `proc.exited` hangs forever if it never calls `terminal.close()`. ### Repro ```js const proc = Bun.spawn([process.execPath, "-e", "console.log('hi')"], { terminal: {}, }); await proc.exited; // process hangs here ``` ### Root cause oven-sh#33882 deferred closing the parent's pty slave fd to `Subprocess::on_process_exit` and added a synchronous drain of the master before the close. That drain calls `reader.read()`, which hits `EAGAIN` (our slave is still open) and re-arms the reader poll via `register_poll()`. Only then do we `close_slave_fd()`, so the reader's EOF (macOS) / `EIO` (Linux) arrives on the *next* epoll tick instead of in the same batch as the pidfd. When that next tick runs after the script's top-level await has already resolved, `on_reader_error` downgrades the Terminal's `JsRef`, but the reader and writer `FilePoll`s are still counted in `loop.active`. Nothing triggers a GC between that downgrade and the next `epoll_wait`, so `Terminal::finalize` (the only path that unregisters those polls) never runs and the process blocks in `epoll_wait` forever. Before oven-sh#33882 the parent's slave fd was closed right after `spawn`, so when the child exited the last slave was already gone and the reader observed `EIO` in the *same* epoll batch as the pidfd. `on_reader_error` downgraded the `JsRef` before the script ended, and the next `onBeforeWait` safepoint finalized the Terminal. Kernel probe on Linux (nonblocking read on master after the last slave closes returns `-1/EIO`, not `0`): ``` === parent keeps slave until EAGAIN then closes === read #0: n=18 data=[hello from child\n] read #1: n=-1 errno=11 (EAGAIN) (closing parent slave now) read #2: n=-1 errno=5 (EIO) ``` ### Fix In `drain_and_close_slave_fd`, after draining and closing `slave_fd`: - call `reader.read()` again so the exit callback fires now (EOF on macOS, `EIO` on Linux) instead of on a later tick when nothing may wake the loop. A grandchild holding the slave keeps this at `EAGAIN` and re-arms, matching the stdout/stderr policy in the same `on_process_exit` body. - `update_ref(false)` on both polls so the event loop can exit regardless; the polls stay registered so grandchild output still arrives while anything else keeps the loop running. - bracket the body with a `ref_()`/`deref_()` scopeguard since both reader callbacks re-enter user JS. `on_reader_done` / `on_reader_error` now gate the exit-callback branch on `READER_DONE` as well as `FINALIZED`, so the second `EIO` dispatch (the still-armed one-shot from the first drain) is a no-op and the exit callback fires exactly once. `#[cfg(unix)]` only; Windows already delivers EOF via `close_pseudoconsole` off-thread. `BufferedReader` / `FileReader` are untouched. ### How did you verify your code works? New test `process exits after subprocess with inline terminal (no terminal.close)` spawns a `bun -e` that runs the repro, sleeps after `child.exited` to give the stale poll a chance to fire, and asserts `{gotOutput: true, exitedSync: true, exitCount: 1, exitCode: 0}`. Fail-before (origin/main `src/` + this test): ``` (fail) ... process exits after subprocess with inline terminal (no terminal.close) [5004.58ms] ^ this test timed out after 5000ms. ``` With the fix: 91 pass / 1 todo / 0 fail. `bun bd test test/js/bun/spawn/spawn.test.ts`: 368 pass, 0 fail. `cargo check -p bun_runtime` clean for `aarch64-apple-darwin` and `x86_64-pc-windows-msvc`. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/terminal/terminal.test.ts <!-- robobun:evidence:end --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
Jul 18, 2026
…n worker terminate (oven-sh#34455) ## What Fixes a heap-use-after-free when a Worker with an in-flight `dns.lookup()` / `dns.resolve*()` is terminated. Surfaced by Node's upstream `test/parallel/test-worker-dns-terminate.js` (being vendored in oven-sh#34441), on the debian 13 x64-asan lane: ``` ==11356==ERROR: AddressSanitizer: heap-use-after-free on address 0x12ce0a3af168 READ of size 4 at 0x12ce0a3af168 thread T6 (Worker) #0 FilePoll::unregister src/io/posix_event_loop.rs:951 #1 FilePoll::deinit_possibly_defer src/io/posix_event_loop.rs:428 #2 FilePoll::deinit_with_vm src/io/posix_event_loop.rs:448 #3 Resolver::on_dns_socket_state src/runtime/dns_jsc/dns.rs:4894 #6 ares_conn_sock_state_cb_update vendor/cares/src/lib/ares_conn.c:36 freed by thread T6 (Worker): drop_in_place<Box<posix_event_loop::Store>> (RareData field drop) VirtualMachine::destroy src/jsc/VirtualMachine.rs:4453 WebWorker::shutdown src/jsc/web_worker.rs:1299 ``` ## Repro ```js const { Worker } = require('worker_threads'); const w = new Worker(` const dns = require('dns'); dns.lookup('nonexistent.org', () => {}); require('worker_threads').parentPort.postMessage('0'); `, { eval: true }); w.on('message', () => w.terminate()); ``` ## Cause `WebWorker::shutdown()` runs, in order: `WebWorker__teardownJSCVM` (frees the `JSGlobalObject`), then `VirtualMachine::destroy()` which drops `rare_data` (frees the `FilePoll` hive `Store`) and finally calls `deinit_runtime_state` which drops `RuntimeState`. That last drop runs `GlobalData::drop` which calls `ares_destroy()` on the per-VM c-ares channel. `ares_destroy()` synchronously fires every pending query callback with `ARES_EDESTRUCTION` and then the socket-state callback for each fd it closes. Those callback chains re-enter: - `Resolver::on_dns_socket_state` -> `FilePoll::deinit_with_vm` on the already-freed hive slot (the ASAN trace above) - `GetAddrInfoRequest::on_cares_complete` -> `DNSLookup::process_get_addr_info` -> `reject_later(global_this)` on the freed `JSGlobalObject` (bmalloc-backed so ASAN misses it) - `ResolveInfoRequest::on_cares_complete` -> `request_completed()` -> `remove_timer()` -> `(*runtime_state()).timer` with the TLS already nulled (null deref) ## Fix Add a `RuntimeHooks::close_dns_for_terminate` slot that runs `Resolver::close_channel_for_terminate()` from `WebWorker::shutdown()` (and the `BUN_DESTRUCT_VM_ON_EXIT` main-thread path) right after `close_all_socket_groups`, while JSC, `RareData.file_polls`, the event loop, and `runtime_state` are all still live. The method also removes the resolver's c-ares timeout timer, which `GetAddrInfoRequest`'s EDESTRUCTION path never unwinds. `GlobalData::drop` still handles the channel if the early hook never ran (it sees `channel == None` when it did). This matches Node's model: `Worker::Exit` -> `CleanupHandles()` closes every handle wrap (including `ChannelWrap`) before disposing the Isolate. ## Verification New ASAN-gated test in `test/js/web/workers/worker-terminate-lifetime.test.ts` spawns four workers that each start a `dns.lookup()` + `dns.resolve4()` and terminates them mid-flight. - **fail-before** (`git stash -- src/ && bun bd test ...`): null-deref panic / ASAN heap-use-after-free - **pass-after**: clean exit 0 across 10 consecutive runs Also verified `test/js/node/dns/` and `test/js/bun/dns/` pass/fail counts are unchanged vs. main, and `bun run rust:check-all` is clean on all targets. <!-- robobun:evidence:begin --> --- **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/web/workers/worker-terminate-lifetime.test.ts <!-- robobun:evidence:end -->
springmin
pushed a commit
that referenced
this pull request
Jul 19, 2026
…-sh#34685) ### What `test/expected-durations.json` drives the LPT bin-packing in `scripts/runner.node.mjs` that assigns test files to `--max-shards` bins. It was last generated 2026-07-07 (builds 69636/69628/69627) and has drifted far enough that the alpine (musl) lanes now have one shard on the critical path roughly 2x the rest: across builds 75488 / 75513 / 75517 / 75562, `:alpine: 3.23 x64` shard 14 runs 8.0-8.4 min while every other shard sits at 3.7-5.5 min. Two things rotted: - oven-sh#33622 moved ~3k `js/{node,bun}/test/parallel/` files into a concurrent phase that logs `[N/M] <path>` without the `--- ` Buildkite group prefix, so `update-test-durations.mjs`' header regex `^--- \[\d+\/\d+\] (.+)$` no longer sees them. The scheduled regen would have silently dropped those ~3k entries. - The three musl lanes have no column of their own and fall back to the debian timings, which are wrong enough on a handful of files to pile them onto one shard. ### Changes **`scripts/update-test-durations.mjs`** - Capture a `musl` column from `linux-x64-musl-alpine-323-test-bun` alongside `default` / `asan` / `windows`. - Match both `[N/M] <path>` and `--- [N/M] <path>` headers, and treat the `--- Running N parallel-safe` banner (oven-sh#34463) as a span delimiter so the last serial test's span does not absorb the concurrent phase. - Concurrent-phase spans are inter-*dispatch* gaps, not wall clock. Clamp spans from bare `[N/M]` headers to 500 ms so the last-dispatched file on each shard cannot absorb the N-wide tail drain or a sibling's 5-15 s retry backoff (without the clamp, nine alphabetically-last `test-zlib-*` files landed at 3-10 s on one lane and ~20 ms on every other). - Reject retry/error headers (`... - code 1`, `... [attempt #2]`) that end after something other than a file extension. The previous table already carried 14 of those as keys; they are harmless to the packer (never matched) but noise in the diff. - Retry 429/5xx from `api.buildkite.com` with `Retry-After` backoff. A burst of 429s mid-run previously aborted the whole regen. - Refresh the stale `release and asan linux-x64 lanes` doc comment. **`scripts/runner.node.mjs`** - Lane selection now maps `--step` values containing `musl` to the new column (the alpine lanes pass `--step=linux-{x64,aarch64}-musl[-baseline]-build-bun`). The existing `entry[lane] ?? default ?? asan ?? windows` fallback chain is extended with `musl` so an entry that only carries a subset of columns still resolves. **`test/expected-durations.json`** - Regenerated from builds 75604 / 75603 / 75596 / 75595 / 75592: 5119 entries, 4 lanes each. (Was 4776 entries, 3 lanes.) **`test/internal/expected-durations.test.ts`** (new) - Guards the table's shape so a future broken regen fails loudly: `_meta.lanes` contains every lane the runner selects and each has >1000 populated entries, every key is a forward-slash relative path ending at a test file extension, the parallel-safe set is present and every one of its values is ≤500 ms, and every value is `{lane: non-negative ms}`. Fails 3/4 against the previous table. ### Verification Replayed the packer (same LPT as `runner.node.mjs`) against the actual per-file timestamps from three builds, two of which (75488, 75517) are not in the source set: | lane | old max/min (75562 / 75517 / 75488) | new max/min | | --- | --- | --- | | musl | 3.08 / 2.74 / 2.90 | **1.30 / 1.40 / 1.55** | | asan | 1.32 / 1.32 / 1.38 | 1.09 / 1.18 / 1.17 | | windows | 1.53 / 1.32 / 1.36 | 1.47 / 1.31 / 1.13 | | default | 1.78 / 2.05 / 1.92 | 1.23 / 1.80 / 1.71 | The musl critical path drops from ~350 s to ~210 s of test wall time. The residual `default` spread on 75517/75488 is one serial test per build running 20-90 s over its median (socket.test.ts at 93 s vs 4.3 s; bundler_compile_splitting at 47 s vs 13 s); no static table can absorb a one-off outlier. This touches only `scripts/` and `test/`, so there is no `src/` stash for the automated fail-before check to exercise; the table-shape test above is the equivalent evidence. ### Follow-up `.buildkite/update-test-durations.yml` uploads the regenerated file as a build artifact and relies on someone committing it. Wiring that schedule up to open a PR (or commit directly) would stop it rotting again; not done here because it needs a write-scoped token on that agent. oven-sh#34552 separately notes the Windows column will want another refresh once the ~1850 re-enabled `itBundled` tests land. <!-- robobun:evidence:begin --> --- **[stamp-90s]** gate passed · iteration 1 · 4 files touched <details><summary>passes on PR (with fix)</summary> ```console Test-only change. Debug/ASAN (expected pass): $ bun bd test 'test/internal/expected-durations.test.ts' $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test test/internal/expected-durations.test.ts info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05) info: component rust-src is up to date info: checking for self-update (current version: 1.29.0) bun test v1.4.0 (5b373f3) test/internal/expected-durations.test.ts: (pass) test/expected-durations.json > every lane the runner selects is declared and populated [173.57ms] (pass) test/expected-durations.json > keys are relative test paths, not runner retry/error labels [228.22ms] (pass) test/expected-durations.json > covers the parallel-safe phase and clamps its spans [279.15ms] (pass) test/expected-durations.json > every entry is {lane: non-negative ms} [436.90ms] 4 pass 0 fail 15 expect() calls Ran 4 tests across 1 file. [3.57s] Exit: 0 ``` </details> <details><summary>diff hotspot</summary> ``` scripts/runner.node.mjs | 10 +- scripts/update-test-durations.mjs | 74 +- test/expected-durations.json | 34536 +++++++++++++++++------------ test/internal/expected-durations.test.ts | 65 + 4 files changed, 20942 insertions(+), 13743 deletions(-) ``` </details> **gate history** · 1 passed · 1 rejected · iteration 1 <details><summary>evidence per changed file</summary> ``` file reads edits tests scripts/runner.node.mjs 5 1 0 scripts/update-test-durations.mjs 2 10 0 test/expected-durations.json 0 0 0 test/internal/expected-durations.test.ts 3 6 0 ``` </details> <!-- robobun:evidence:end -->
springmin
pushed a commit
that referenced
this pull request
Jul 20, 2026
…ven-sh#34693) ## Use-after-free in `H2FrameParser::on_native_writable` Fleet ASAN fuzz hit (p-h2c cleartext harness, seed 1, `server-conn.recv.*:A8`): ``` use-after-poison READ 8 (shadow f7 = user poison, HiveArray slot re-poison) #0 Vec::len (write_buffer) #2 has_backpressure h2_frame_parser.rs:3276 #3 on_native_writable h2_frame_parser.rs:9605 #4 NewSocket<true>::on_writable socket_body.rs:894 #8 us_internal_ssl_on_writable bun-usockets openssl.c:1851 allocated by: HiveArray Fallback<H2FrameParser,256>, H2FrameParser::constructor ``` ### Cause `on_native_writable` loops `flush()` and checks `has_backpressure()` between iterations. `flush()` re-enters JS via `flush_stream_queue` -> `dispatch_write_callback` / `onStreamEnd` / `onWantTrailers`. A callback that destroys the session reaches `detach_native_callback`, dropping the socket's `+1` on the parser. If that was the last external ref, `flush()`'s own keepalive is all that remains and drops on return, so the next `has_backpressure()` reads a HiveArray slot that was just `drop_in_place`'d and re-poisoned by `POOL.put`. `on_native_read` already takes a `keepalive()` for exactly this reason (h2_frame_parser.rs:9590); `on_native_writable` did not. In release builds there is no poison: the same ordering is a silent use-after-free in every `node:http2` server/client on a native socket. The read of a stale `write_buffer.len()` can satisfy the loop condition and send the next `flush()` into UAF writes on the freed parser. ### Fix - Take a `keepalive()` for the extent of `on_native_writable`, mirroring `on_native_read`. - `NativeCallbacks::on_data`/`on_writable`: copy the raw `*mut H2FrameParser` out of the enum before dispatching, so the `JsCell<NativeCallbacks>` borrow does not span a re-entrant `detach_native_callback` that overwrites the cell. ### Test `test/js/node/http2/node-http2-writable-destroy-fixture.ts` reproduces the exact fleet stack under ASAN by faulting `send`/`writev` to 0 (backpressure, arms WRITABLE), queuing a DATA frame whose write callback runs `session.destroy()` + `Bun.gc(true)`, then clearing the fault so the writable event drains the queue inside `on_native_writable`. Added to `node-http2-syscall-fault.test.ts` as an ASAN-gated subprocess test. <details><summary>Fail-before ASAN report (matches the fleet hit)</summary> ``` ==ERROR: AddressSanitizer: use-after-poison on address 0x... READ of size 8 at 0x... thread T0 #2 H2FrameParser::has_backpressure h2_frame_parser.rs:3276:33 #3 H2FrameParser::on_native_writable h2_frame_parser.rs:9605:21 #4 NativeCallbacks::on_writable socket_body.rs:3960:20 #5 NewSocket<false>::on_writable socket_body.rs:894:39 allocated by thread T0 here: ... Fallback<H2FrameParser, 256>::new_boxed hive_array.rs:668 ... H2FrameParser::constructor h2_frame_parser.rs:9800 SUMMARY: AddressSanitizer: use-after-poison ... Vec<u8>::len ``` </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 2 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/node/http2/node-http2-syscall-fault.test.ts <!-- robobun:evidence:end --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
Jul 29, 2026
…ed (oven-sh#36247) ## What `test/js/bun/http/bun-serve-html.test.ts` segfaults on `windows-aarch64` after oven-sh#36175 landed (builds 84162, 84194; one earlier sighting in 83933): ``` panic(main thread): Segmentation fault at address 0x48 Features: ... dev_server(14) ... ``` Symbolicated in oven-sh#36214 as `AsyncFSTask<Access>::run_from_js_thread` with `self = null`, i.e. a zeroed `ConcurrentTask` was dispatched. ## Cause `DevServer.watcher_atomics.events[*].concurrent_task` is the intrusive MPSC node the watcher thread links into `EventLoop.concurrent_tasks` when it submits a hot-reload event. It was an inline field of `DevServer`, so `server.stop()` → `drop(Box<DevServer>)` freed it while it was still linked. The next `tick_concurrent` then read `.next`/`.task`/`.auto_delete` from freed memory. ASAN on Linux confirms: ``` heap-use-after-free: ConcurrentTask::get_next (unbounded_queue.rs) ← BatchIterator::next ← EventLoop::tick_concurrent_with_count freed by: Box<DevServer>::drop ← NewServer::deinit_if_we_can ← NewServer::stop ← dispose_from_js (using server) ``` On release builds the freed block reads back as zeros, so the copied `Task` is `{tag: 0, ptr: null}`; tag 0 is `task_tag::Access`, whose `run_from_js_thread` loads `self.result` at offset `0x48`. The bug is latent and platform-agnostic. oven-sh#36175 exposed it because the CI runner now spawns the napi addon prebuild in the background while serial tests run; that writes under the watched project root, so the `jsx-runtime` DevServers in this test file now reliably receive a hot-reload event between the last `await fetch` and `using server` disposal. ## Fix `watcher_atomics` is now a `NonNull<WatcherAtomics>` owned via `bun_core::heap::into_raw`, so the allocation can outlive `DevServer` and every queued pointer keeps allocation-root provenance. `watcher_acquire_event`, `watcher_release_and_submit_event` and `recycle_event_from_dev_server` take `*mut Self` and derive the returned `*mut HotReloadEvent` (and the linked `concurrent_task` node) from that root pointer via raw place projections rather than from a `&mut WatcherAtomics` reborrow. `Drop for DevServer` reads `next_event` after `Watcher::shutdown` has serialised out the watcher thread (which guarantees it is stable): - `DONE`: nothing is queued; clear and `heap::destroy` as before. - otherwise: a `concurrent_task` is still linked (or its `Task` is already in the drain FIFO). Null `owner` on every event and leave the allocation alive. `HotReloadEvent::run` checks `owner.is_null()` first; when set it reclaims the allocation via the new `atomics` backref and returns without touching the dead `DevServer`. The `# Safety` contracts on `run` and the `BakeHotReloadEvent` dispatch arm are updated to describe the null-owner case. ## Test `test/js/bun/http/bun-serve-html-hot-reload-drop.test.ts` creates a development server, bundles once so `app.js` is watched, synchronously rewrites `app.js`, spins briefly without yielding so the watcher thread can enqueue, disposes the server, then yields. Ten iterations. In a separate file because the React-bundling cases in `bun-serve-html.test.ts` already exceed the default per-test timeout under a debug+ASAN build on `main`. <details><summary>fail-before (debug+ASAN, src/ at main)</summary> ``` ==25521==ERROR: AddressSanitizer: heap-use-after-free on address 0x79315e4743e8 READ of size 8 at 0x79315e4743e8 thread T0 #2 <ConcurrentTask as Node>::get_next unbounded_queue.rs:82 #3 BatchIterator<ConcurrentTask>::next unbounded_queue.rs:135 #4 EventLoop::tick_concurrent_with_count event_loop.rs:507 0x79315e4743e8 is located 488 bytes inside of 16512-byte region freed by thread T0 here: #9 Box<DevServer>::drop #12 NewServer<false,true>::deinit_if_we_can mod.rs:1770 #13 NewServer<false,true>::stop mod.rs:1665 #14 NewServer<false,true>::dispose_from_js server_body.rs:2584 ``` </details> Passes with the fix in ~2.4s under debug+ASAN (also on a local `windows-aarch64` debug build, where the original `bun-serve-html.test.ts` is now 19/19); `test/bake/deinitialization.test.ts` still green. Supersedes the producer half of oven-sh#36214 (which adds a sentinel for the same zeroed-task symptom). <!-- robobun:evidence:begin --> --- **[review]** gate passed · iteration 4 · 6 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 1 failed, 2 skipped $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/http/bun-serve-html.test.ts test/js/bun/http/bun-serve-html-hot-reload-drop.test.ts bun test v1.4.0 (5f6622f) test/js/bun/http/bun-serve-html.test.ts: waitForServer /tmp/html-css-js_ObkyZk { "/": "/tmp/html-css-js_ObkyZk/index.html", "/dashboard": "/tmp/html-css-js_ObkyZk/dashboard.html", } [0.12ms] bundle index.html 1.09 KB [0.05ms] bundle dashboard.html 1.27 KB (pass) serve html [630.67ms] waitForServer /tmp/bun-serve-html-txt_5C6B7a { "/": "/tmp/bun-serve-html-txt_5C6B7a/index.html", } [0.15ms] bundle index.html 0.40 KB HASH efbnbska (pass) serve plugins > basic plugin [556.20ms] waitForServer /tmp/html-css-js-failing-plugin_OPRhwb { "/": "/tmp/html-css-js-failing-plugin_OPRhwb/index.html", } error: Plugin failed intentionally at /tmp/html-css-js-failing-plugin_OPRhwb/styles.css:0 error: Plugin failed intentionally at /tmp/html-css-js-failing-plugin_OPRhwb/styles.css:0 (pass) serve plugins > serve html with failing plugin [491.35ms] waitForServer /tmp/html-css-js-empty-plugins_biqnN6 { "/": "/tmp/htm ... (truncated) release without fix: all passed bun test v1.4.0-canary.1 (96ff7ec) test/js/bun/http/bun-serve-html.test.ts: waitForServer /tmp/html-css-js_ZmgkBG { "/": "/tmp/html-css-js_ZmgkBG/index.html", "/dashboard": "/tmp/html-css-js_ZmgkBG/dashboard.html", } [0.00ms] bundle index.html 1.09 KB [0.00ms] bundle dashboard.html 1.27 KB (pass) serve html [25.17ms] waitForServer /tmp/bun-serve-html-txt_uWNogT { "/": "/tmp/bun-serve-html-txt_uWNogT/index.html", } [0.00ms] bundle index.html 0.40 KB HASH efbnbska (pass) serve plugins > basic plugin [17.25ms] waitForServer /tmp/html-css-js-failing-plugin_Kd1a8p { "/": "/tmp/html-css-js-failing-plugin_Kd1a8p/index.html", } error: Plugin failed intentionally at /tmp/html-css-js-failing-plugin_Kd1a8p/styles.css:0 error: Plugin failed intentionally at /tmp/html-css-js-failing-plugin_Kd1a8p/styles.css:0 (pass) serve plugins > serve html with failing plugin [16.33ms] waitForServer /tmp/html-css-js-empty-plugins_Ecz7qJ { "/": "/tmp/html-css-js-empty-plugins_Ecz7qJ/index.html", } [0.00ms] bundle index.html 0.71 KB (pass) serve plugins > empty plugin array [13.23ms] Waiting for server waitForServer /tmp/html-css-js-concurrent-plugins_l7wQg5 { "/": "/tmp/ ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: 2 skipped $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/http/bun-serve-html.test.ts test/js/bun/http/bun-serve-html-hot-reload-drop.test.ts bun test v1.4.0 (5f6622f) test/js/bun/http/bun-serve-html.test.ts: waitForServer /tmp/html-css-js_CpXhx4 { "/": "/tmp/html-css-js_CpXhx4/index.html", "/dashboard": "/tmp/html-css-js_CpXhx4/dashboard.html", } [0.09ms] bundle index.html 1.09 KB [0.05ms] bundle dashboard.html 1.27 KB (pass) serve html [588.64ms] waitForServer /tmp/bun-serve-html-txt_rjZL4e { "/": "/tmp/bun-serve-html-txt_rjZL4e/index.html", } [0.15ms] bundle index.html 0.40 KB HASH efbnbska (pass) serve plugins > basic plugin [552.11ms] waitForServer /tmp/html-css-js-failing-plugin_D8NJ2O { "/": "/tmp/html-css-js-failing-plugin_D8NJ2O/index.html", } error: Plugin failed intentionally at /tmp/html-css-js-failing-plugin_D8NJ2O/styles.css:0 error: Plugin failed intentionally at /tmp/html-css-js-failing-plugin_D8NJ2O/styles.css:0 (pass) serve plugins > serve html with failing plugin [505.55ms] waitForServer /tmp/html-css-js-empty-plugins_vK0DVI { "/": "/tmp/htm ... (truncated) release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 673ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/7] gen bake.{client,server,error}.js -> bake.client.js, bake.server.js, bake.error.js [2/7] gen generated_host_exports.rs generated_host_exports.rs: 94 exports (host=3, lazy=10, generic=81, rust=0); 240 extern-C blocks audited [2/7] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu) nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19) �[1m�[92m Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) �[1m�[92m Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno) �[1m�[92m Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr) �[1m�[92m Compiling�[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys) �[1m�[92m Compiling�[0m bun_safety v0.0.0 (/workspace/bun/src/safety) �[1m�[92m Compiling�[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys) �[1m�[92m Compiling�[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys) �[1m�[92m Compiling�[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd) �[1m�[92m Compiling�[0m ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/runtime/bake/DevServer.rs | 63 +++- src/runtime/bake/dev_server/lifecycle.rs | 12 +- src/runtime/bake/dev_server/mod.rs | 401 +++++++++++---------- src/runtime/dispatch.rs | 10 +- .../http/bun-serve-html-hot-reload-drop.test.ts | 82 +++++ test/js/bun/http/bun-serve-html.test.ts | 10 +- 6 files changed, 374 insertions(+), 204 deletions(-) ``` </details> **gate history** · 3 passed · 2 rejected · iteration 4 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/runtime/bake/DevServer.rs 10 13 0 src/runtime/bake/dev_server/lifecycle.rs 5 9 0 src/runtime/bake/dev_server/mod.rs 13 12 0 src/runtime/dispatch.rs 3 2 0 test/js/bun/http/bun-serve-html-hot-reload-drop.test.ts 1 5 0 test/js/bun/http/bun-serve-html.test.ts 6 9 0 ``` </details> <!-- robobun:evidence:end --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
springmin
pushed a commit
that referenced
this pull request
Jul 29, 2026
oven-sh#36310) ### What does this PR do? Fixes `test/js/node/net/node-net-server.test.ts > should listen on unix domain socket` going red on the alpine 3.23 lanes since oven-sh#36175 (seen on main builds 84293, 84503, 84549, 84601 and ~60 branch builds). Every test in the `net.createServer listen` block armed `setTimeout(closeAndFail, 100)` next to `server.listen()`. That timer was never testing `listen()` itself: `Bun.listen` binds synchronously and `'listening'` is scheduled via `setTimeout(emitListeningNextTick, 1, this)`, so the 100 ms race was against the test process's own scheduling latency. The runner already bounds each test, so the extra timer only added a flake surface (and hid the real error behind `function should not have been called`). oven-sh#36175 didn't touch `net` or this file, but it moved the allowlisted files into a single batch, so the handful of remaining serial files (this one is in `excludeFiles`) now run much earlier in the shard. On alpine that lands while the docker-service coordinator is still bringing up the mysql containers in the background: ``` t=233226 [9/282] node-net-server.test.ts t=233436 ✗ should listen on unix domain socket [144.58ms] ← 100 ms timer fired t=234907 coordinator: mysql_native_password ready ← docker init finished 1.5 s later t=242361 [attempt #2] node-net-server.test.ts 21 pass ← same file green once docker is idle ``` (from build 84601, alpine 3.23 x64 shard `019fab6d-27b7-4c39`) ### Change Remove the 100 ms `setTimeout(closeAndFail, ...)` from the nine listen tests and route `server.on('error', ...)` to `done(err)` so a real bind failure reports its actual error. Same assertions, same code paths (`listen()` → `'listening'` → `server.address()` checks); only the hand-rolled deadline that duplicated the test runner's timeout is gone. The 500 ms timers in the `events` block are untouched; they guard real client↔server round trips, have `is_done` guards, and haven't flaked. ### How did you verify your code works? - `bun bd test test/js/node/net/node-net-server.test.ts` → 21 pass / 0 fail. - Reproduced the race locally by running the file under background CPU+disk load (4× `yes`, 2 GB `dd`): with the old timer the listen block failed 1/5 runs at `function should not have been called`; with this change 5/5 runs pass under the same load (including a 246 ms `'listening'` that would have tripped the old 100 ms timer). `node-tls-server.test.ts` has the same 100 ms pattern and is also a serial `excludeFiles` entry; happy to fold it in here if preferred, but it hasn't been observed red so I kept this scoped to the reported file. <!-- robobun:evidence:begin --> --- **[stamp-90s]** gate passed · iteration 1 · 1 files touched <details><summary>passes on PR (with fix)</summary> ```console Test-only change. Debug/ASAN (expected pass): $ bun bd test 'test/js/node/net/node-net-server.test.ts' $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test test/js/node/net/node-net-server.test.ts bun test v1.4.0 (6b920f8) test/js/node/net/node-net-server.test.ts: (pass) net.createServer listen > should throw when no port or path when using options [27.65ms] (pass) net.createServer listen > should listen on IPv6 by default [153.42ms] (pass) net.createServer listen > should listen on IPv4 [26.37ms] (pass) net.createServer listen > should call listening [19.34ms] (pass) net.createServer listen > should provide listening property [22.93ms] (pass) net.createServer listen > should listen on localhost [17.90ms] (pass) net.createServer listen > should listen on localhost [17.45ms] (pass) net.createServer listen > should listen without port or host [24.19ms] (pass) net.createServer listen > should listen on unix domain socket [19.40ms] (pass) net.createServer listen > should bind IPv4 0.0.0.0 when listen on 0.0.0.0, issue#7355 [31.32ms] (pass) net.createServer events > should receive data [159.16ms] (pass) net.createServer events > should call end [155.39ms] (pass) net.createServer events > should call close [19.65ms] (pass) net.createServer events > should call connection and drop [70.90ms] (pass) net.createServer events > should error on an invalid port [23.08ms] (pass) net.createServer events > should call abort with signal [25.98ms] (pass) net.createServer events > should echo data [111.91ms] (pass) net.createServer events > oven-sh#8374 [72.98ms] (pass) accepted socket event-loop hold matches Node (per-connection KeepAlive) > server.stop() + accepted socket.unref() lets the process exit [318.84ms] (pass) accepted socket event-loop hold matches Node (per-connection KeepAlive) > server.unref() alone does not drop a ref'd accepted connection's hold [1548.16ms] (pass) accepted socket event-loop hold matches Node (per-connection KeepAlive) > half-open accepted sockets after peer FIN do not busy-poll the event loop (Windows AFD DISCONNECT) [4 ... (truncated) Exit: 0 ``` </details> <details><summary>diff hotspot</summary> ``` test/js/node/net/node-net-server.test.ts | 117 +++++++------------------------ 1 file changed, 25 insertions(+), 92 deletions(-) ``` </details> **gate history** · 2 passed · 0 rejected · iteration 1 <details><summary>evidence per changed file</summary> ``` file reads edits tests test/js/node/net/node-net-server.test.ts 2 3 0 ``` </details> <!-- robobun:evidence:end -->
springmin
pushed a commit
that referenced
this pull request
Aug 2, 2026
) ## What `JSSink::assign_to_stream` now detaches the freshly created `JSReadable*SinkController` (nulling its `m_sinkPtr`) when the C++ stream-pump setup returns an error, before returning to the caller. ## Why The generated `${name}__assignToStream` functions create the controller with `m_sinkPtr = sinkPtr` and then call into `GlobalObject::assignToStream` → `readDirectStream` / `readStreamIntoSink`. If that setup throws (for example a direct `ReadableStream` whose `pull` getter throws), the controller is never started, so nothing ever calls `end()`/`close()` to null `m_sinkPtr`. The caller's error path (`Writable::init` for `Bun.spawn`) then releases and frees the native sink. When the controller is later swept, its destructor runs `${name}__controllerDetached` / `${name}__finalize` on freed memory. ASAN report: ``` heap-use-after-free on address 0x799feed81c78 READ of size 1 #0 JSSink<FileSink>::js_controller_detached Sink.rs:567 #1 FileSink__controllerDetached generated_jssink.rs:179 #2 JSReadableFileSinkController::~JSReadableFileSinkController() freed by: #12 FileSink::deinit FileSink.rs:1142 oven-sh#16 Writable::pipe_release Writable.rs:70 oven-sh#17 Writable::init Writable.rs:339 oven-sh#18 spawn_maybe_sync js_bun_spawn_bindings.rs:1379 ``` The fix is at the generic `JSSink::assign_to_stream` layer so it covers every sink type (`FileSink`, `NetworkSink`, `FetchRequestBodySink`, ...), not just the spawn path. ## Repro ```js const { openSync, closeSync } = require("node:fs"); const fd = openSync("/tmp/out.txt", "w"); let armed = false; const stream = new ReadableStream({ type: "direct", get pull() { if (armed) throw new Error("pull unavailable"); return () => {}; }, }); armed = true; try { Bun.spawn({ cmd: [process.execPath, "-e", "0"], stdio: [stream, fd, "ignore"] }); } catch {} closeSync(fd); Bun.gc(true); // sweep -> controller dtor -> UAF ``` ## Tests The two existing `spawn.test.ts` cases that cover the stdin-stream-setup-throws path now force a full GC in the child fixture so the controller destructor runs deterministically under debug+ASAN as well. Previously they were only failing on the release-asan lane (where the whole file has been quarantined as `[ASAN] [TIMEOUT]`), which is why this went unnoticed. ``` bun bd test test/js/bun/spawn/spawn.test.ts -t "stdin stream setup fails" ``` fails on `main` (ASAN heap-use-after-free in the child's stderr) and passes with this change. `spawn-stdin-readable-stream-edge-cases.test.ts` and `body-stream.test.ts` continue to pass.
springmin
pushed a commit
that referenced
this pull request
Aug 5, 2026
… sink ends inline (oven-sh#36939) ### Crash Sentry [BUN-3BZF](https://bun-p9.sentry.io/issues/?query=BUN-3BZF) (2,975 events since 2026-05-25, macOS-dominant): `Panic: called Option::unwrap() on a None value` at `FetchTasklet::callback`'s `task_ref.http.as_mut().unwrap()`, reached from the HTTP thread's result dispatch (`us_internal_ssl_on_data -> HTTPClient::fail -> dispatch_result_and_reset -> AsyncHTTP::on_async_http_callback_raw -> FetchTasklet::callback`). `http` is set once at creation and cleared only at deinit, so the panic means the callback ran against a freed `FetchTasklet`. ### Cause `start_request_stream` takes a `+1` on the tasklet that must be released exactly once by `write_end_request`. For a native `ByteStream` request body (an upstream response body piped into `fetch()`), `wire_native_sink` installs the sink's `source` handle *before* any of its `EndedInline` returns (`ReadableStream.rs:328` vs `:337/:352/:359`), so a stream that picked up an error or its last chunk between `fetch()` and the `can_stream` tick comes back `EndedInline` with a native source attached. The `EndedInline` arm released the `+1` (via `write_end_request`) but left `self.sink` installed with `ended == false`. Every terminal path then runs `cancel_request_body_sink`, which saw a "live" native sink and took its native arm: `abort_task()` plus a second `write_end_request` — releasing the same `+1` again. The double release collapses the refcount while the other owners (the JS-side initial ref and the HTTP thread's in-flight ref) still use the tasklet. Under ASAN the deterministic form is the trace below (deinit runs inside `cancel_request_body_sink`, then `on_progress_update` keeps using `self`). In release builds the same imbalance frees the tasklet while it is still in use (or double-frees, handing a live tasklet's block back to the allocator), which surfaces as downstream crashes in the fetch completion path — the BUN-3BZF unwrap is the tasklet's `http` field read from freed/recycled memory. ``` READ of size 8 ... core::mem::replace::<bun_jsc::js_promise::Strong> #2 FetchTasklet::on_progress_update FetchTasklet.rs:1158 freed by thread T0 here: #12 FetchTasklet::deinit FetchTasklet.rs:509 oven-sh#16 FetchTasklet::write_end_request FetchTasklet.rs:2281 oven-sh#17 FetchTasklet::cancel_request_body_sink FetchTasklet.rs:2368 oven-sh#18 FetchTasklet::on_progress_update FetchTasklet.rs:1143 ``` ### Fix Leave the sink in the same state `end_from_stream` (the normal native termination) leaves it: `ended = true`, source and task detached. The terminal `cancel_request_body_sink` then hits its existing `if sink.ended { return }` guard and cannot release the ref a second time (it also no longer spuriously aborts a request whose body simply ended inline). ### Verification - New fixture `fetch-stream-body-ended-inline-fixture.ts` drives the window: an upstream server that advertises a larger `content-length` than it sends and closes a few ms later, piped as the body of a TLS `fetch()` (the handshake keeps the wire-attempt window open), 100 iterations. - Unfixed debug+ASAN build: heap-use-after-free with the trace above, 8/8 runs. - Fixed build: `bun bd test test/js/web/fetch/fetch-abort-stream-body.test.ts` passes (5 pass, 1 pre-existing skip), including the new test. - `test/js/web/fetch/body-stream.test.ts`: 9086 pass / 0 fail. `fetch.test.ts` and `fetch.stream.test.ts`: identical pass/fail counts to an unfixed baseline in the same container (the failures are pre-existing network/timeout issues). - The test is `skipIf(!isASAN)`: the release build corrupts silently, so only sanitizer lanes can observe the failure.
springmin
pushed a commit
that referenced
this pull request
Aug 6, 2026
…e cache (oven-sh#37034) ### Problem On the `13 x64-asan` lane, a test that exercises non-ISO Temporal calendars from a test callback can abort after a fully green run with a LeakSanitizer report. Seen in build 89504 on oven-sh#37024, whose `test/js/bun/bun-object/deep-equals-temporal.test.ts` uses `[u-ca=hebrew]`: ``` Direct leak of 624 byte(s) in 1 object(s) allocated from: #1 icu_75::HebrewCalendar::clone() const #2 icu_75::Calendar::createInstance(icu_75::TimeZone*, icu_75::Locale const&, UErrorCode&) #3 ucal_open_75 #4 JSC::TemporalCore::buildCalendarTemplate(WTF::AbstractLocker const&, unsigned int) #5 JSC::TemporalCore::withCalendar<JSC::TemporalCore::calendarYear(...)::$_0>(...) ``` The CI annotation titles this `direct leak of 624b in {closure#0} (src/jsc/JSValue.rs:1664:22)` because that is the first in-repo frame (the test-runner's `JSValue::call`); everything below it is WebKit/ICU. ### Cause `TemporalCore::withCalendar` (`vendor/WebKit/.../temporal/core/CalendarICUBridge.cpp`) keeps up to 8 open `UCalendar` templates in a process-lifetime `LazyNeverDestroyed` `TinyLRUCache`, one per calendar ID (non-ISO arithmetic, plus pure-ISO `PlainDateTime.prototype.with`, which reaches the same path unguarded); LRU eviction `ucal_close`s them, so the set is bounded. The `CalendarCacheEntry` that owns each `UCalendar` is `WTF_MAKE_TZONE_ALLOCATED` (bmalloc), which LSan does not scan, so the libc-allocated `UCalendar` (and the ICU `TimeZone` inside it) is reported as a direct leak even though it is reachable. Whether a given run aborts depends on whether some stale stack or register value still points at the ICU object when LSan scans at exit, hence the intermittence. This is the calendar twin of the already-suppressed `TemporalCore::withTimeZone` entry (same cache design, same TZone-allocated owner). ### Fix - Add a `leak:TemporalCore::buildCalendarTemplate` suppression to `test/leaksan.supp`, mirroring the `withTimeZone` entry. The pattern anchors on the template builder rather than `withCalendar` itself so that a future real leak inside one of the many op lambdas `withCalendar` runs would still be reported; every cached-template allocation carries the builder frame. (`withTimeZone` has no such builder frame, its `ucal_open` is inline, so that entry keeps its existing pattern.) - Drop the `test/no-validate-leaksan.txt` escape hatch oven-sh#37024 added for `deep-equals-temporal.test.ts`, re-enabling leak validation for it; that file exercises the suppressed path on the asan lane. ### Verification On a debug ASAN build, running `bun test test/js/bun/bun-object/deep-equals-temporal.test.ts` under the CI leak-validation env (`BUN_DESTRUCT_VM_ON_EXIT=1`, `detect_leaks=1:abort_on_error=1`, repo suppression file): - with the new entry: clean exit, 5/5 runs - without it: LSan abort with the calendar-template stacks above, 3/3 runs A standalone probe exercising 8 non-ISO calendars plus pure-ISO `PlainDateTime.with` from a timer callback shows the same split (10/10 aborts without, 10/10 clean with; `print_suppressions=1` attributes exactly the ICU template allocations to the new entry). Top-level module code cannot reproduce this: its allocation stacks carry `JSC::JSModuleLoader::evaluateNonVirtual`, which the suppression file already covers wholesale. An ASAN-gated test pinning the entry was part of an earlier revision and was dropped per review; the re-enabled `deep-equals-temporal.test.ts` covers the path in CI instead. The Expect-wrapper shutdown leak mentioned in the dropped no-validate comment is a separate issue tracked in oven-sh#32180: that is `bun test`'s own finalizer-owned memory, while this cache deliberately survives VM teardown, so oven-sh#32180 would not prevent this report. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 1 · docs-only change; test-proof not applicable <!-- robobun:evidence:end --> --------- Co-authored-by: Dylan Conway <dylan.conway567@gmail.com>
springmin
pushed a commit
that referenced
this pull request
Aug 8, 2026
…-comparison (oven-sh#37168) ### Problem `Bun__deepEquals` has heap-use-after-free when a getter on a nested object mutates one of the objects being compared. All entry points are affected: `Bun.deepEquals`, `expect().toEqual` / `toStrictEqual`, `assert.deepStrictEqual` / `deepEqual`, and `util.isDeepStrictEqual`. ```js // Malloc=1 <bun-asan> repro.mjs const p1 = {}, p2 = {}; for (let i = 0; i < 8; i++) { p1['k'+i] = i; p2['k'+i] = i; } let f = 0; p1.a = { get x() { if (!f++) for (let i = 0; i < 2000; i++) p1['n'+i] = i; return 1; } }; p2.a = { get x() { return 1; } }; p1.z = 1; p2.z = 1; Bun.deepEquals(p1, p2, true); ``` ASAN (with `Malloc=1` so JSC's bmalloc routes through the system allocator): ``` heap-use-after-free READ of size 8 #0 CompactPropertyTableEntry::key() Structure.h #1 PropertyTable::forEachProperty #2 Structure::forEachProperty #3 Bun__deepEquals<...> bindings.cpp freed by: PropertyTable::destroyIndexVector <- PropertyTable::rehash <- PropertyTable::add <- Structure::addNewPropertyTransition <- JSObject::putDirectInternal ``` ### Cause The object fast path walks the structure's `PropertyTable` with `Structure::forEachProperty` and recurses into `Bun__deepEquals` from inside the lambda. Comparing a nested value can run a user getter; if that getter adds (or deletes) properties on the parent object, JSC takes the shared table off the old structure and rehashes it, freeing the index vector the outer walk is iterating. Every remaining sibling property is then read from freed memory and its stale offset fed to `getDirect()`. In release builds this shows up as a SEGV at a forged address or a wrong verdict. ### Fix Collect the (left, right) value pairs into a `MarkedArgumentBuffer` under `forEachProperty` with no side effects, then run `sameValue` and the recursive comparisons after the walk finishes. This is the same shape as `Object.assign`'s fast path (snapshot under `forEachProperty`, side-effectful work after). The buffer keeps the snapshotted values visible to GC, so allocation churn in a getter cannot collect them either. The reverse `o2` walk already did only direct structure reads and now also completes before any user code can run. Verdicts are unchanged for non-mutating comparisons (existing suites pass); a comparison whose getter mutates the object now deterministically compares the snapshot, which matches Node's behavior for the repro above (`true`). ### Verification - New test in `test/js/bun/bun-object/deep-equals.test.ts` (renamed from `deep-equals.spec.ts` to match the test naming convention): spawns an ASAN child with `Malloc=1` (bmalloc routed through the system allocator so ASAN can see the freed table) covering same-structure, mixed-structure, delete, right-side mutation, and GC-churn variants across all entry points. Fails before the fix (ASAN heap-use-after-free abort), passes after. - `test/js/bun/bun-object/`, `test/js/node/assert/deep-equal.test.ts`, `assert-typedarray-deepequal.test.ts`: 542 pass. - `test/js/bun/test/expect.test.js`: 415 pass. - `test/js/node/test/parallel/test-assert-deep-with-error.js`: 2 pass. ### Scope The same pattern exists in `JSC__JSValue__forEachPropertyImpl` in this file (the `Bun.inspect` / `console.log` property walk), where the formatter callback can run a nested value's `inspect.custom` mid-walk. Verified with ASAN to hit the same free/read pair. That is a pre-existing bug in the console/inspect subsystem and is intentionally excluded here; a follow-up fix for that site is in progress. The other `forEachProperty` sites (CommonJS export enumeration, HTTP header writing, the ordered/non-indexed iteration variants) run no user code inside the walk. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 3 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/bun-object/deep-equals.test.ts <!-- robobun:evidence:end -->
springmin
pushed a commit
that referenced
this pull request
Aug 8, 2026
…id-format (oven-sh#37169) ### Problem `Bun.inspect` and `console.log` have a heap-use-after-free when formatting a value runs user code that mutates the object being formatted. The default-enabled `Symbol.for("nodejs.util.inspect.custom")` hook on a nested value is enough to trigger it: ```js // Malloc=1 <bun-asan> repro.mjs const p = {}; for (let i = 0; i < 8; i++) p['k'+i] = i; let f = 0; p.a = { [Symbol.for('nodejs.util.inspect.custom')]() { if (!f++) for (let i = 0; i < 256; i++) p['n'+i] = i; return 'a'; } }; p.z = 1; console.log(Bun.inspect(p).length); ``` ASAN (with `Malloc=1` so JSC's bmalloc routes through the system allocator): ``` heap-use-after-free READ of size 8 #0 CompactPropertyTableEntry::key() Structure.h #1 PropertyTable::forEachProperty #2 Structure::forEachProperty #3 JSC__JSValue__forEachPropertyImpl bindings.cpp freed by: PropertyTable::destroyIndexVector <- PropertyTable::rehash <- PropertyTable::add <- Structure::addNewPropertyTransition <- JSObject::putDirectInternal ``` ### Cause The fast path of `JSC__JSValue__forEachPropertyImpl` walks the structure's `PropertyTable` with `Structure::forEachProperty` and invokes the formatter callback from inside the walk. The callback recursively formats the property value, which can run user code: a nested value's `inspect.custom`, or a getter on a built-in subclass (for example an overridden `Map.prototype.size`). If that code adds or deletes properties on the parent object, JSC rehashes the shared table, freeing the index vector the outer walk is iterating, and every remaining entry is read from freed memory. The fast-path guard only inspects the parent's structure, and a parent with plain data properties passes it; the hostile hook lives on a nested value. Same bug class as the deepEquals fix in oven-sh#37168, which deliberately excluded this site. ### Fix Collect the entries (key, attributes, direct value) under `forEachProperty` with no side effects, then resolve remaining values and invoke the callback on the snapshot after the walk finishes. The values go in a `MarkedArgumentBuffer` so GC in a callback cannot collect them; keys are retained as `Identifier`s. The snapshot is per structure walk, so the prototype-chain restart loop still re-reads each prototype's live structure. Properties added to the object while it is being formatted are no longer printed: the walk now reflects the object as it was when formatting started. That matches Node, which collects the key list before formatting values. The other `forEachProperty` sites are unaffected: the non-indexed and ordered variants never take this fast path, and the remaining callers run no user code in the callback. ### Verification - New test in `test/js/bun/util/inspect.test.js` (ASAN-only, child spawned with `Malloc=1`) covering: `inspect.custom` adding properties via `Bun.inspect` and `console.log`, deleting properties, a `Map` subclass `size` getter, the prototype fast-walk of an own-property-less object, and GC churn inside the hook with object-valued siblings formatted afterwards. Fails before the fix (ASAN abort, empty stdout), passes after. - `test/js/bun/util/inspect.test.js`: 74 pass. `test/js/bun/console/`: 85 pass, 1 skip. - `inspect-error.test.js` minified-file snapshots and `inspect-error-leak.test.js` fail identically with and without this diff locally (pre-existing, unrelated to property enumeration).
springmin
pushed a commit
that referenced
this pull request
Aug 10, 2026
…ven-sh#37273) ### Problem `H2FrameParser::handle_received_stream_id` creates a `Stream` box, inserts it into the stream map, and then invokes the JS `streamStart` callback directly via `callback.call` without arming the `DispatchGuard`, while still holding the raw `*mut Stream`. Every other JS dispatch site in the parser arms the guard, because `rewrite_read` frees streams queued in `pending_engine_stream_closes` only at dispatch depth 0. JS reached from inside that callback (the `Http2Stream` constructor calls `this.on("pause", ...)`, so a patched `EventEmitter.prototype.on` runs there; the handler also calls back into native `rstStream` for refused streams) can close the just-created stream, queueing its deferred free, and then re-enter `parser.read()` at depth 0. The drain frees the box, and the callback return path writes the stream context through the dangling pointer: ``` ==ERROR: AddressSanitizer: heap-use-after-free ... #1 <bun_runtime::api::h2_frame_parser_body::Stream>::set_context src/runtime/api/bun/h2_frame_parser.rs:2110 #2 <...H2FrameParser>::handle_received_stream_id src/runtime/api/bun/h2_frame_parser.rs:5372 #3 <...H2FrameParser>::get_next_stream src/runtime/api/bun/h2_frame_parser.rs:8335 freed by: #12 <...H2FrameParser>::rewrite_read::{closure#3} src/runtime/api/bun/h2_frame_parser.rs:5804 ``` The callers that keep dereferencing the returned pointer (`request()`, `get_next_stream`, the engine HEADERS path) were exposed to the same freed box. ### Fix Arm `enter_dispatch` across the callback, matching the invariant documented on `enter_dispatch` (every section that holds a `Stream` pointer while user JS can run must arm the guard). With the guard armed, the deferred-close drain cannot run while the callback executes, so the pointer stays valid for `set_context` and for the callers. Also skip the context install when the callback closed the stream: `free_resources` already dropped its `sctx` root, and re-inserting one afterwards would pin the dead JS stream object until the session dies. This is the guard-arming fix for the pre-existing issue flagged during review of oven-sh#37272 (that PR only removes dead code around it). ### Verification New test in `test/js/node/http2/node-http2-streams-rehash.test.ts` (the file covering this class of reentrancy bugs) reproduces the exact sequence: close the new stream and re-enter `read()` from inside the `streamStart` callback. Without the fix it fails on every build tier: heap-use-after-free under the ASAN debug build, and on release builds `getStreamContext(2)` throws "Invalid stream id" because the drain already freed the entry inside the callback. With the fix the entry survives the callback with no context installed (covering the skip-install branch), and a follow-up depth-0 `read()` asserts the deferred close then actually drains. Existing http2 suites (`node-http2.test.js`, `h2-conformance.test.ts`, the staged h2 tests, node's server-push parallel tests) pass with the change. <!-- robobun:evidence:begin --> --- **[review]** gate passed · iteration 1 · 2 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 1 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" "test/js/node/http2/node-http2-streams-rehash.test.ts" bun test v1.4.0 (8f79562) test/js/node/http2/node-http2-streams-rehash.test.ts: (pass) session.request() from a stream 'timeout' listener during forEachStream does not UAF on hashmap rehash [3284.09ms] (pass) http2 client request() does not hold *Stream across user-controlled options getters [6184.76ms] 198 | env: bunEnv, 199 | stdout: "pipe", 200 | stderr: "pipe", 201 | }); 202 | const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); 203 | expect({ stdout: stdout.trim(), exitCode, stderr }).toMatchObject({ stdout: "OK", exitCode: 0 }); ^ error: expect(received).toMatchObject(expected) { - "exitCode": 0, - "stdout": "OK", + "exitCode": 1, + "stderr": + "================================================================= + ==101685==ERROR: AddressSanitizer: heap-use-after-free on address 0x79be9bb005c0 at pc 0x00000e7ec22e bp ... (truncated) release without fix: all passed bun test v1.4.0-canary.1 (7725ac8) test/js/node/http2/node-http2-streams-rehash.test.ts: (pass) session.request() from a stream 'timeout' listener during forEachStream does not UAF on hashmap rehash [163.99ms] (pass) http2 client request() does not hold *Stream across user-controlled options getters [78.42ms] (pass) closing the new stream and re-entering read() inside the streamStart callback does not UAF [31.29ms] (pass) http2 client write callback that opens new streams during flushQueue does not UAF [49.40ms] (pass) DeferredTaskQueue::run tolerates an on_auto_flush callback that unregisters itself and returns true [46.51ms] 5 pass 0 fail 5 expect() calls Ran 5 tests across 1 file. [513.00ms] __F:0:S:0 ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" "test/js/node/http2/node-http2-streams-rehash.test.ts" bun test v1.4.0 (8f79562) test/js/node/http2/node-http2-streams-rehash.test.ts: (pass) session.request() from a stream 'timeout' listener during forEachStream does not UAF on hashmap rehash [3278.34ms] (pass) http2 client request() does not hold *Stream across user-controlled options getters [6171.66ms] (pass) closing the new stream and re-entering read() inside the streamStart callback does not UAF [1906.06ms] (pass) http2 client write callback that opens new streams during flushQueue does not UAF [2819.28ms] (pass) DeferredTaskQueue::run tolerates an on_auto_flush callback that unregisters itself and returns true [2618.43ms] 5 pass 0 fail 5 expect() calls Ran 5 tests across 1 file. [19.19s] __F:0:S:0 release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 689ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/6] gen generated_host_exports.rs generated_host_exports.rs: 93 exports (host=3, lazy=10, generic=80, rust=0); 239 extern-C blocks audited [1/6] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu) nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19) �[1m�[92m Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) �[1m�[92m Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno) �[1m�[92m Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr) �[1m�[92m Compiling�[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys) �[1m�[92m Compiling�[0m bun_safety v0.0.0 (/workspace/bun/src/safety) �[1m�[92m Compiling�[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys) �[1m�[92m Compiling�[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys) �[1m�[92m Compiling�[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd) �[1m�[92m Compiling�[0m bun_picohttp v0.0.0 (/workspace/bun/src/picohttp) �[1m�[92m Compiling�[0m bun_brotli v ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/runtime/api/bun/h2_frame_parser.rs | 19 +++- .../node/http2/node-http2-streams-rehash.test.ts | 100 +++++++++++++++++++++ 2 files changed, 115 insertions(+), 4 deletions(-) ``` </details> **gate history** · 2 passed · 0 rejected · iteration 1 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/runtime/api/bun/h2_frame_parser.rs 10 4 0 test/js/node/http2/node-http2-streams-rehash.test.ts 2 3 0 ``` </details> <!-- robobun:evidence:end --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
Aug 10, 2026
…ven-sh#37273) ### Problem `H2FrameParser::handle_received_stream_id` creates a `Stream` box, inserts it into the stream map, and then invokes the JS `streamStart` callback directly via `callback.call` without arming the `DispatchGuard`, while still holding the raw `*mut Stream`. Every other JS dispatch site in the parser arms the guard, because `rewrite_read` frees streams queued in `pending_engine_stream_closes` only at dispatch depth 0. JS reached from inside that callback (the `Http2Stream` constructor calls `this.on("pause", ...)`, so a patched `EventEmitter.prototype.on` runs there; the handler also calls back into native `rstStream` for refused streams) can close the just-created stream, queueing its deferred free, and then re-enter `parser.read()` at depth 0. The drain frees the box, and the callback return path writes the stream context through the dangling pointer: ``` ==ERROR: AddressSanitizer: heap-use-after-free ... #1 <bun_runtime::api::h2_frame_parser_body::Stream>::set_context src/runtime/api/bun/h2_frame_parser.rs:2110 #2 <...H2FrameParser>::handle_received_stream_id src/runtime/api/bun/h2_frame_parser.rs:5372 #3 <...H2FrameParser>::get_next_stream src/runtime/api/bun/h2_frame_parser.rs:8335 freed by: #12 <...H2FrameParser>::rewrite_read::{closure#3} src/runtime/api/bun/h2_frame_parser.rs:5804 ``` The callers that keep dereferencing the returned pointer (`request()`, `get_next_stream`, the engine HEADERS path) were exposed to the same freed box. ### Fix Arm `enter_dispatch` across the callback, matching the invariant documented on `enter_dispatch` (every section that holds a `Stream` pointer while user JS can run must arm the guard). With the guard armed, the deferred-close drain cannot run while the callback executes, so the pointer stays valid for `set_context` and for the callers. Also skip the context install when the callback closed the stream: `free_resources` already dropped its `sctx` root, and re-inserting one afterwards would pin the dead JS stream object until the session dies. This is the guard-arming fix for the pre-existing issue flagged during review of oven-sh#37272 (that PR only removes dead code around it). ### Verification New test in `test/js/node/http2/node-http2-streams-rehash.test.ts` (the file covering this class of reentrancy bugs) reproduces the exact sequence: close the new stream and re-enter `read()` from inside the `streamStart` callback. Without the fix it fails on every build tier: heap-use-after-free under the ASAN debug build, and on release builds `getStreamContext(2)` throws "Invalid stream id" because the drain already freed the entry inside the callback. With the fix the entry survives the callback with no context installed (covering the skip-install branch), and a follow-up depth-0 `read()` asserts the deferred close then actually drains. Existing http2 suites (`node-http2.test.js`, `h2-conformance.test.ts`, the staged h2 tests, node's server-push parallel tests) pass with the change. <!-- robobun:evidence:begin --> --- **[review]** gate passed · iteration 1 · 2 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 1 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" "test/js/node/http2/node-http2-streams-rehash.test.ts" bun test v1.4.0 (8f79562) test/js/node/http2/node-http2-streams-rehash.test.ts: (pass) session.request() from a stream 'timeout' listener during forEachStream does not UAF on hashmap rehash [3284.09ms] (pass) http2 client request() does not hold *Stream across user-controlled options getters [6184.76ms] 198 | env: bunEnv, 199 | stdout: "pipe", 200 | stderr: "pipe", 201 | }); 202 | const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); 203 | expect({ stdout: stdout.trim(), exitCode, stderr }).toMatchObject({ stdout: "OK", exitCode: 0 }); ^ error: expect(received).toMatchObject(expected) { - "exitCode": 0, - "stdout": "OK", + "exitCode": 1, + "stderr": + "================================================================= + ==101685==ERROR: AddressSanitizer: heap-use-after-free on address 0x79be9bb005c0 at pc 0x00000e7ec22e bp ... (truncated) release without fix: all passed bun test v1.4.0-canary.1 (7725ac8) test/js/node/http2/node-http2-streams-rehash.test.ts: (pass) session.request() from a stream 'timeout' listener during forEachStream does not UAF on hashmap rehash [163.99ms] (pass) http2 client request() does not hold *Stream across user-controlled options getters [78.42ms] (pass) closing the new stream and re-entering read() inside the streamStart callback does not UAF [31.29ms] (pass) http2 client write callback that opens new streams during flushQueue does not UAF [49.40ms] (pass) DeferredTaskQueue::run tolerates an on_auto_flush callback that unregisters itself and returns true [46.51ms] 5 pass 0 fail 5 expect() calls Ran 5 tests across 1 file. [513.00ms] __F:0:S:0 ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" "test/js/node/http2/node-http2-streams-rehash.test.ts" bun test v1.4.0 (8f79562) test/js/node/http2/node-http2-streams-rehash.test.ts: (pass) session.request() from a stream 'timeout' listener during forEachStream does not UAF on hashmap rehash [3278.34ms] (pass) http2 client request() does not hold *Stream across user-controlled options getters [6171.66ms] (pass) closing the new stream and re-entering read() inside the streamStart callback does not UAF [1906.06ms] (pass) http2 client write callback that opens new streams during flushQueue does not UAF [2819.28ms] (pass) DeferredTaskQueue::run tolerates an on_auto_flush callback that unregisters itself and returns true [2618.43ms] 5 pass 0 fail 5 expect() calls Ran 5 tests across 1 file. [19.19s] __F:0:S:0 release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 689ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/6] gen generated_host_exports.rs generated_host_exports.rs: 93 exports (host=3, lazy=10, generic=80, rust=0); 239 extern-C blocks audited [1/6] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu) nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19) �[1m�[92m Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) �[1m�[92m Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno) �[1m�[92m Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr) �[1m�[92m Compiling�[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys) �[1m�[92m Compiling�[0m bun_safety v0.0.0 (/workspace/bun/src/safety) �[1m�[92m Compiling�[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys) �[1m�[92m Compiling�[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys) �[1m�[92m Compiling�[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd) �[1m�[92m Compiling�[0m bun_picohttp v0.0.0 (/workspace/bun/src/picohttp) �[1m�[92m Compiling�[0m bun_brotli v ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/runtime/api/bun/h2_frame_parser.rs | 19 +++- .../node/http2/node-http2-streams-rehash.test.ts | 100 +++++++++++++++++++++ 2 files changed, 115 insertions(+), 4 deletions(-) ``` </details> **gate history** · 2 passed · 0 rejected · iteration 1 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/runtime/api/bun/h2_frame_parser.rs 10 4 0 test/js/node/http2/node-http2-streams-rehash.test.ts 2 3 0 ``` </details> <!-- robobun:evidence:end --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
springmin
pushed a commit
that referenced
this pull request
Aug 13, 2026
…n-sh#37813) ### Problem - An HTML route served without the DevServer (`development: false` or `{ hmr: false }`) bundles on its first request. If the only client disconnects and `server.stop(true)` is called while a `[serve.static]` plugin still has that build parked, `stop()` settles, the next GC frees the server, and the build then finishes against the freed server. - Debug build: `AddressSanitizer: heap-use-after-free` in `html_bundle::Route::on_complete` (parked in `onLoad`) or `Route::on_plugins_resolved` (parked in the plugin's `setup()`). A release build reads the freed `NewServer` with no report. - Cause: while a route is building, nothing counts as keeping the server alive. The clients waiting on the build only count as connections, so once they drop the server's idle check sees no pending work. - The other route kinds already count their asynchronous work in the server's pending-request counter; the HTML build was the one piece of in-flight work that did not. ### Fix - Entering the building state now takes one pending request on the server; both ways out of it (build finished, plugin load rejected) answer the waiting clients and then release it. - This holds the server for exactly the window in which the route will call back into it. The release runs the server's idle pass, so a server stopped mid-build is freed right after the build lands. - Visible change: `server.pendingRequests` is 1 while an HTML route bundles and `await server.stop()` waits for the bundle, as it already does for a `fetch` handler still running. A build cancelled by VM teardown is not covered; at exit it leaves the same state an in-flight `fetch` handler does. - Verification: a new test parks the route in the build, in the plugin load, and in a plugin load that rejects. Unfixed debug build: all three report 0 pending requests and an early-settled `stop()`, and the first two die with the ASAN reports above. Fixed: all three pass, as do the existing HTML-serve tests that do not need the DevServer. ### Background - HTML routes: `Bun.serve({ routes: { "/": html } })` with an imported `.html` file. Without the DevServer the route bundles the page once, on the first request, registers the outputs as static routes, and holds requests that arrive during the build. - `[serve.static]` plugins: a bunfig entry naming bundler plugins for these routes, loaded on the first request. Both the plugin load and the bundle finish on later event-loop turns and complete by calling back into the server through a raw pointer stored on the route. - Pending requests: the server's count of in-flight work, exposed as `server.pendingRequests`. `stop()` settles and the server can be torn down only when the count is zero; static and file routes already raise it when a response goes asynchronous. - Server lifetime: stopping a server does not free it. Once nothing is pending, the JS wrapper becomes collectable and the native server is freed on a later GC, so a stale pointer to it only fails after a GC. <details> <summary>Original description</summary> ### Repro HTML route served without the DevServer (`development: false` or `{ hmr: false }`), with a `[serve.static]` plugin whose `onLoad` parks on a promise. Request the route, drop the client, `server.stop(true)`, drop the server, `Bun.gc(true)` plus a couple of event-loop turns, then let `onLoad` resolve. Debug (ASAN) build: ``` ==1165==ERROR: AddressSanitizer: heap-use-after-free on address 0x73defa800738 ... READ of size 8 at 0x73defa800738 thread T0 #0 in <bun_runtime::server::NewServer<false, false>>::global_this src/runtime/server/mod.rs:451 #1 in <bun_runtime::server::AnyServer>::global_this src/runtime/server/mod.rs:3847 #2 in <bun_runtime::server::html_bundle::Route>::on_complete src/runtime/server/HTMLBundle.rs #3 in JSBundleCompletionTask::on_complete src/runtime/api/js_bundle_completion_task.rs:642 freed by thread T0 here: ... #11 in <bun_runtime::server::NewServer<false, false>>::deinit src/runtime/server/mod.rs:2122 #12 in NewServer::schedule_deinit::{closure#1} src/runtime/server/mod.rs:1957 ``` Parking in the plugin's `setup()` instead (so the route is still waiting for the plugin load when the server goes away) gives the same report one step earlier: ``` READ of size 1 ... in <bun_runtime::server::html_bundle::Route>::on_plugins_resolved src/runtime/server/HTMLBundle.rs #1 in <bun_runtime::server::server_body::ServePlugins>::handle_on_resolve src/runtime/server/server_body.rs:1150 #2 in bun_runtime::server::server_body::on_resolve_impl ``` On a release build the same sequence reads a freed `NewServer` (its config, then `append_static_route` / `reload_static_routes` on it) without a report. ### Cause `html_bundle::Route` keeps a raw `server` back-pointer and bundles on its first request. Both the plugin load and the build finish on later event-loop turns and call back into the server through that pointer (`on_plugins_resolved` reads the config, `on_complete` registers the output files as static routes and reloads the route table). While the route is in `State::Building`, nothing holds the server on its behalf: `on_plugins_resolved` only refs the route itself, and the clients waiting in `pending_responses` only count as connections, which they can drop at any time. So once the last client disconnects and the server is stopped, `deinit_if_we_can` sees no pending requests, settles `stop()`, downgrades the wrapper, and the next GC frees the `NewServer` with the build still in flight. `StaticRoute` / `FileRoute` / `DirectoryRoute` already handle their asynchronous work with the server's `pending_requests` counter (`on_pending_request` when a response goes async, `on_static_request_complete` when it finishes); the route's build is the same kind of in-flight work and was the one thing not counted. ### Fix `schedule_bundle` calls `server.on_pending_request()` whenever the route enters `State::Building` (plugins ready, or plugins still loading), and the two ways out of that state (`on_complete`, `on_plugins_rejected`) go through a new `finish_building`, which answers the pending responses and then calls `on_request_complete()`. That keeps the server allocated for exactly the window in which the route will call back into it, and `on_request_complete` runs the idle pass, so a server that was stopped while building is downgraded and freed right after the build lands (the `stop()` promise now settles then as well, matching what happens for a `fetch` handler that is still running when `stop()` is called). With that invariant, `on_complete` no longer needs its `Option` handling of the back-pointer; it takes the server once at the top, the same way `on_plugins_resolved` already did. A visible consequence: `server.pendingRequests` is 1 while an HTML route is bundling, and `await server.stop()` waits for the bundle. A build whose plugin never settles therefore keeps the server allocated, as an unsettled `fetch` handler already does. Not covered: a build cancelled by VM teardown never reaches `Route::on_complete` (the completion task returns early on `cancelled`), so at exit the route keeps its ref and, now, its pending request; that is the same state an in-flight `fetch` handler leaves a server in at exit and nothing observes it. The DevServer's own plugin wait uses a different back-pointer and is not changed here. ### Verification `test/js/bun/http/bun-serve-html-build-holds-server.test.ts` (separate small file; `bun-serve-html.test.ts` is too slow under the debug ASAN build for a lifetime test, as `bun-serve-html-hot-reload-drop.test.ts` notes). One fixture, parked in turn in the build (`onLoad`), in the plugin load (`setup()`), and in a plugin load that then rejects (the `on_plugins_rejected` exit has to release the request too). Each child reports `server.pendingRequests` while parked, whether `stop(true)` settled across ten event-loop turns before the route was released, and whether the wrapper became collectable afterwards; the test expects `{ pendingRequestsWhileParked: 1, stopBeforeRelease: "pending", collectedAfterwards: true }` plus a clean exit. If `stop()` did settle early, the fixture lets the server get collected before releasing the route, which is the sequence above. Unfixed debug build: all three report `pendingRequestsWhileParked: 0, stopBeforeRelease: "settled"`, and the first two children die with the ASAN reports above (the rejection case has no use-after-free to hit; it fails on the report). Fixed: the three pass in under a second each. Also run on the fixed debug build: `bun-serve-html-405.test.ts`, `bun-serve-html-hot-reload-drop.test.ts`, `test/bake/serve-plugins-dev-server.test.ts` (all pass), and `bun-serve-html.test.ts`, where everything that does not need the DevServer passes, including `serve plugins > concurrent requests to multiple routes during plugin load`; its `development: true` cases fail in this container with `EMFILE while initializing file watcher for development server` (inotify instance limit) before reaching any of this code. </details>
springmin
pushed a commit
that referenced
this pull request
Aug 22, 2026
oven-sh#39947) ### Problem - A worker whose entry point goes through a package.json `imports` or `exports` map leaks 12 KiB (3 `PathBuffer`s, more on Windows) when its thread exits. On an ASAN build LeakSanitizer reports `Direct leak of 12288 byte(s)` allocated in `module_bufs` (`src/resolver/package_json.rs`), reached from `resolve_entry_point_specifier` on the worker thread. - Cause: `MODULE_BUFS` is a thread local `Cell<*mut ModuleBufs>` with nothing that frees the box. The resolver's other per thread buffers (`BufsSlot` in `resolver.rs`, `LazyPathBuf` in `bun_paths`) got a destructor in oven-sh#30875. This one did not. ### Fix - Wrap the pointer in `ModuleBufsSlot`, whose `Drop` destroys the box when the thread exits. Same shape as `BufsSlot`. Access is unchanged, so the recursion notes on the thread local still hold, and the static TLS template is still one pointer. - Correct because the destructor runs when the thread's TLS is torn down, after every resolver frame on that thread has returned. The main thread's box lives for the process, as before. - Verified: `test/js/web/workers/worker-entry-point.test.ts` (new file) runs a worker through an `imports` alias in a child with `detect_leaks=1`. It fails on main with the report above and passes with this change (checked both ways with a debug build). `test/js/bun/binary/tls-segment-size.test.ts` still passes. ### Background - The resolver keeps a few large scratch buffers per thread instead of on the stack. They are boxed on first use and only a pointer sits in TLS, so the TLS segment stays small on every platform. - A worker thread resolves its own entry point and preloads, so it is the common short lived thread that touches these buffers. The bundler's pool threads live as long as the pool. - The ASAN CI lanes run test children with `detect_leaks=1`. The test sets that itself (plus the repo's `test/leaksan.supp`) so that a local ASAN build checks it too. A build without ASAN ignores the options and checks the behaviour only. <details><summary>Notes</summary> Found through oven-sh#39811, whose worker test resolves an `imports` alias and failed on the ASAN lanes because of this leak. oven-sh#39811 carries this change until this lands and is otherwise independent of it. oven-sh#35060 (overflow bundle threads, open) includes the same change as one of its hunks, because its threads are short lived too. The case is in its own file, for the worker entry point resolution cases, rather than in `worker.test.ts`: three of that file's stress cases go over their budget on a debug build on a slow machine, which would hide whether this case itself flips. oven-sh#39811 adds its worker case to the same file. Without `print_suppressions=0` LeakSanitizer prints a "Suppressions used" table to stderr on exit when an unrelated, suppressed allocation exists in the process, so the test passes that along with the suppressions file when the environment does not already set `LSAN_OPTIONS`. </details> <!-- robobun:evidence:begin --> --- **[review]** gate passed · iteration 1 · 2 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 1 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/web/workers/worker-entry-point.test.ts bun test v1.4.0 (4199361) test/js/web/workers/worker-entry-point.test.ts: 41 | LSAN_OPTIONS: 42 | bunEnv.LSAN_OPTIONS ?? 43 | `print_suppressions=0:suppressions=${path.join(import.meta.dir, "..", "..", "..", "leaksan.supp")}`, 44 | }, 45 | ); 46 | expect(stderr).toBe(""); ^ error: expect(received).toBe(expected) - "" + " + ================================================================= + ==385090==ERROR: LeakSanitizer: detected memory leaks + + Direct leak of 12288 byte(s) in 1 object(s) allocated from: + #0 0x000007dd95c8 in malloc crtstuff.c + #1 0x00000be19934 in std::sys::alloc::unix::alloc /root/.rustup/toolchains/nightly-2026-07-20-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/alloc/unix.rs:31:18 + #2 0x00000be184b9 in <std::alloc::System>::alloc_impl /root/.rustup/toolchains/nightly-2026-07-20-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/alloc.rs:149:78 + #3 ... (truncated) release without fix: all passed bun test v1.4.0-canary.1 (2e16ac4) test/js/web/workers/worker-entry-point.test.ts: (pass) package.json imports alias as the entry point > the worker runs and its thread exits without leaking [11.86ms] 1 pass 0 fail 3 expect() calls Ran 1 test across 1 file. [218.00ms] __F:0:S:0 ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/web/workers/worker-entry-point.test.ts bun test v1.4.0 (4199361) test/js/web/workers/worker-entry-point.test.ts: (pass) package.json imports alias as the entry point > the worker runs and its thread exits without leaking [3743.31ms] 1 pass 0 fail 3 expect() calls Ran 1 test across 1 file. [6.05s] __F:0:S:0 release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 667ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [0/5] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu) nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19) �[1m�[92m Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) �[1m�[92m Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno) �[1m�[92m Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr) �[1m�[92m Compiling�[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys) �[1m�[92m Compiling�[0m bun_safety v0.0.0 (/workspace/bun/src/safety) �[1m�[92m Compiling�[0m bun_base64 v0.0.0 (/workspace/bun/src/base64) �[1m�[92m Compiling�[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys) �[1m�[92m Compiling�[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys) �[1m�[92m Compiling�[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd) �[1m�[92m Compiling�[0m bun_picohttp v0.0.0 (/workspace/bun/src/picohttp) �[1m�[92m Compiling�[0m bun_brotli v0.0.0 (/workspace/bun/src/brotli) �[1m�[92m Compiling�[0m bun_outpu ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/resolver/package_json.rs | 23 +++++++++--- test/js/web/workers/worker-entry-point.test.ts | 50 ++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 5 deletions(-) ``` </details> **gate history** · 1 passed · 1 rejected · iteration 1 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/resolver/package_json.rs 2 2 0 test/js/web/workers/worker-entry-point.test.ts 1 2 0 ``` </details> **root cause** · written by the author bot With --target bun or node, the resolver short-circuits node:, bun: and hardcoded builtin specifiers into an external result whose primary path is the bare specifier rather than an absolute file path, and entry-point resolution passed that through, so enqueue_entry_item either tripped the absolute-path assert, reported a misleading "File not found", or for bun:wrap collided with the runtime's pre-registered source and left the build with no entry points. The fix marks entry-point resolutions with their own ImportKind so the resolver no longer applies externalization rules to them, and resolv… <!-- robobun:evidence:end -->
springmin
pushed a commit
that referenced
this pull request
Aug 23, 2026
…40064) ### Problem - GitHub closes only the first reference after a keyword, so "Fixes #1, #2" leaves #2 open. "Supersedes #3" links nothing, and no reference closes a pull request. - The last 1000 merged PRs name 274 such references. PR oven-sh#32292 is open although merged oven-sh#36135 says "Supersedes oven-sh#32292". ### Fix - `.github/workflows/close-linked-issues.yml` runs on `pull_request_target` `closed` (a merge into the default branch of `oven-sh/bun`) and on `workflow_dispatch` with a PR number and `dry_run`. Everything is inline in one `actions/github-script` step, with no checkout. - Each open target is closed as `completed` with the comment "Closed as completed by #N." or "Superseded by #N.". Closed or missing targets, the PR itself and other repositories are skipped. - The parser has no regex. A closing keyword (close, fix, resolve, supersede, replace, any tense) must lead the reference, alone or in a list. A negated, hedged or noun keyword, or one whose subject is another reference, does not count ("may fix", "the rm fix #1", "oven-sh#100 supersedes #1"). - Verified: `test/internal/close-linked-issues.test.ts` (333 cases) runs the YAML's script against fake `github`, `context` and `core`. Also the 1000-PR parse (Notes). ### Background - GitHub's own keywords are close, fix and resolve (-s, -ed). Each links one reference, and only a merge into the default branch closes it. - `pull_request_target` runs in the base repository with a write token, also for fork PRs. That is safe only when no PR-controlled code runs. Here the description is the only PR input, parsed as text. <details><summary>Notes</summary> A close through the API does not create the "closed this in #N" timeline link that GitHub makes for its own closes. The comment carries the PR number instead. How the parser was calibrated. I pulled the descriptions of the last 1000 merged PRs and listed every line with a keyword next to a reference. The keyword families, list shapes and reference forms in the script are the ones that appear there. A reference is `#1`, `owner/repo#1`, an issue or pull URL (bare or in `<>`), or a markdown link. Four lines would have been wrong with a plain keyword-then-reference rule, and each led to a rule: - "the open `rm` fix oven-sh#37521" (oven-sh#38379): "fix" as a noun. Base forms (fix, close, resolve, supersede, replace) count only at the start of a sentence or line, or after will, should, does, and, and a few similar words. "to" is not one of them ("unable to fix #1", "how to fix #1"). - "May also fix oven-sh#12318 / oven-sh#10046, untested" (oven-sh#38242): hedged. may, might, could, would, partially and the negations disqualify the keyword, looking past adverbs such as "also". - "Supersedes the closed oven-sh#26040" (oven-sh#36289) and "a comment on closed oven-sh#35351" (oven-sh#35365): "closed" as an adjective. A determiner or preposition before the keyword disqualifies it. - "supersedes oven-sh#33130's optimisation" (oven-sh#35843): a number that continues into a word is not a reference. Review added: a reference before the keyword is the subject ("oven-sh#100 supersedes #1"), also through "which" or "that" ("reverts oven-sh#100, which fixed #1") and across a removed span ("oven-sh#100 ~~also~~ fixes #1"). A hedge two words before the keyword disqualifies it ("hopefully this fixes #1", "could this fix #1?"). A clause that starts with if, when, once, until or unless is not a statement. The tokenizer keeps a line break as a token so that "Fixes #1" on one line and "Fixes #2" on the next stay two statements. Code spans, fences, indented code, blockquotes, HTML comments and strikethrough are skipped. The block stripping follows CommonMark for fences (also inside a blockquote), indented code, blockquotes with lazy continuation, setext underlines and HTML comments, and GFM for `~~` flanking. Result over the 1000 descriptions: 274 distinct references in 135 PRs. I checked the current state of all of them through GraphQL. All but one are closed (202 issues completed, 5 duplicates, 66 pull requests). The one open target is PR oven-sh#32292, superseded by merged oven-sh#36135. No open target is a false positive. Every review change kept this result. Patterns that are deliberately not handled: a bulleted list under "Closes:" on its own line (not seen in the sample), references separated by whitespace only ("#1 #2"), "fix for #1", and GH-1 style references. A `?` after the list is not treated as a question. The block parser tracks no list containers, so a second paragraph of a list item indented by four spaces is read as an indented code block and skipped. A removed span or inline comment reads as one word, so "Fixes <!-- n --> #1" finds nothing. The test suite covers: the phrases above, stopping at the right place in real sentences, CRLF descriptions, URLs with fragments or a `/files` suffix, case-insensitive `Owner/Repo#1`, the fake API where a lookup, an update or a comment fails, the `dry_run` input, an invalid `pr_number` input, an unmerged PR, a PR merged into a non-default branch, the merge event body against a later edit, and a description with no closing statement. The first revision of this PR checked out the repository and ran `scripts/close-linked-issues.ts`. Jarred asked for no checkout and no script file, so the script moved inline into the workflow and the test now reads it out of the YAML. </details> <!-- robobun:evidence:begin --> --- **[stamp-90s]** gate passed · iteration 9 · 2 files touched <details><summary>passes on PR (with fix)</summary> ```console Test-only change. Debug/ASAN (expected pass): $ bun bd test 'test/internal/close-linked-issues.test.ts' $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test test/internal/close-linked-issues.test.ts bun test v1.4.1 (4448a2e) test/internal/close-linked-issues.test.ts: (pass) finds "Fixes oven-sh#39852" [176.21ms] (pass) finds "Closes oven-sh#31772. Fixes oven-sh#31771." [22.28ms] (pass) finds "- Fixes oven-sh#39930" [12.28ms] (pass) finds "Fixes: oven-sh#30429" [10.46ms] (pass) finds "FIXES #1" [7.86ms] (pass) finds "(Fixes #1)" [8.97ms] (pass) finds "**Fixes #1**" [10.20ms] (pass) finds "__Fixes #1__" [9.83ms] (pass) finds "_Fixes #1_" [11.25ms] (pass) finds "Fixes **#1**" [9.72ms] (pass) finds "**Fixes** #1" [7.13ms] (pass) finds "**Fixes:** #1" [8.11ms] (pass) finds "Fixes #1 and **#2**" [11.47ms] (pass) finds "Fixes **#1**, **#2**" [9.13ms] (pass) finds "## Why (fixes oven-sh#13771, closes oven-sh#30543)" [16.08ms] (pass) finds "Closes oven-sh#11418" [19.46ms] (pass) finds "Resolves #1. Resolved #2. Resolve #3." [12.09ms] (pass) finds "Fixes oven-sh#34055, oven-sh#30327, oven-sh#24394, oven-sh#20816, oven-sh#32403, oven-sh#11898, oven-sh#10056." [17.11ms] (pass) finds "Fixes oven-sh#18192 and oven-sh#31675 as a consequence" [10.45ms] (pass) finds "Fixes #1, #2, and #3" [10.96ms] (pass) finds "Fixes #1 & #2" [7.63ms] (pass) finds "Closes oven-sh#33280, Closes oven-sh#32864 and Closes oven-sh#29696 (the timer in oven-sh#32949 is orthogonal)" [20.29ms] (pass) finds "Closes oven-sh#33182 and oven-sh#32947 on top of current main (which already has oven-sh#36304 for catalogs)." [16.12ms] (pass) finds "Fixes #1,\n#2" [7.76ms] (pass) finds "Fixes #1, #2,\nand #3" [9.27ms] (pass) finds "Fixes #1\nand #2" [8.57ms] (pass) finds "Fixes #1\n& #2" [6.80ms] (pass) finds "Fixes #1 and\n#2" [7.31ms] (pass) finds "Supersedes oven-sh#39908 (same change, moved from a fork branch)" [13.21ms] (pass) finds "Supersedes oven-sh#38778 and oven-sh#38391. Carries the entry point arm of oven-sh#35053." [14.43ms] (pass) finds "Supersedes oven-sh#39193 and keeps its three tests." [11.48ms] (pass) finds "This supersedes oven-sh#33306 and oven-sh#32803. Their tests are kept here." [13.73ms] (pass) finds "- This replaces oven-sh#33793. Its ... (truncated) Exit: 0 ``` </details> <details><summary>diff hotspot</summary> ``` .github/workflows/close-linked-issues.yml | 950 ++++++++++++++++++++++++++++++ test/internal/close-linked-issues.test.ts | 598 +++++++++++++++++++ 2 files changed, 1548 insertions(+) ``` </details> **gate history** · 29 passed · 0 rejected · iteration 9 <details><summary>evidence per changed file</summary> ``` file reads edits tests .github/workflows/close-linked-issues.yml 6 12 0 test/internal/close-linked-issues.test.ts 3 11 0 ``` </details> <!-- robobun:evidence:end -->
springmin
pushed a commit
that referenced
this pull request
Sep 1, 2026
…safe functions (oven-sh#39848) ### Problem - An addon's `napi_finalize` segfaults under `napi_body::Finalizer::run`: Sentry BUN-4MXA and BUN-4NJZ on 1.4.0, and oven-sh#41055 (libsql under `bun test --parallel`). - `NapiRef::callFinalizer` (`napi.h`) copied the finalizer into the task that runs after the GC, so `napi_delete_reference` in between did not cancel it and it ran on freed memory. Node dequeues it. - `ThreadSafeFunction::destroy` (`napi_body.rs`) freed the function before its finalizer ran. After `napi_tsfn_abort` it also waited for every other thread's reference (oven-sh#40671), so a holder that never releases pinned the process. ### Fix - `NapiEnv::m_pendingRefFinalizers` (Node's `pending_finalizers`): the GC adds the ref and queues a drain if the set was empty. Each drain takes the first ref, requeues if any remain, then runs its finalizer. `~NapiRef` removes itself, so a deleted reference never finalizes. - `ThreadSafeFunction::finalize` (was `destroy`) runs from the closing dispatch whatever `thread_count` is: the finalizer first, then the JS-thread release under the lock. It frees the allocation if no thread reference is left, else the last release does (Node's `Finalize` + `MaybeDelete`). - Verified: four new tests in `test/napi/napi.test.ts`, compared with Node 26. ### Background - A `NapiRef` backs a `napi_ref`. `napi_wrap` creates one that holds the JS object weakly, and JSC calls it when it sweeps the object. - Outside `NAPI_EXPERIMENTAL` a finalizer may not run inside the GC, so it is queued on the event loop. Node never finalizes a reference deleted before then. - Node finalizes a threadsafe function on the JS thread when it closes and deletes it after the callback, or at the last `thread_count` release. <details><summary>Notes</summary> Branch history. The first version kept a `HashSet` of refs with a queued task each; the maintainer push on this branch replaced it with the `ListHashSet` drain (`NapiEnv::drainOneRefFinalizer`, one ref per event-loop task, the next drain queued before the finalizer runs so a finalizer that deletes or enqueues other refs is plain) and merged oven-sh#40671 (threadsafe function: finalize on abort without waiting for the other threads, `finalize` replaces `destroy`, `env_teardown_done` renamed `resources_released`). That PR is closed in favour of this one. A `napi_wrap` without a result keeps the copying `callFinalizer()` path: its runtime-owned reference (`NapiRefSelfDeletingWeakHandleOwner`) is deleted right after, so nothing can delete it while the copy is queued. The four tests: `napi_wrap` and `napi_add_finalizer` references deleted in the same turn as the GC, a parent finalizer that deletes children collected by the same GC (in both creation orders), a tsfn finalizer that uses its handle, and (from oven-sh#40671) a tsfn aborted while another thread still holds a reference. The last one waits for the holder thread by deadline (3 s, the holder gives up after 2 s) so it stays under the default test timeout. Fail before, on release 1.4.0 and on an unfixed ASAN debug build. The fixture's native objects are static and record a finalizer that runs after the delete instead of reading freed memory, so the failure is a clean output mismatch with Node: ``` - napi_wrap: collected before delete: true, finalized after delete: 0 - napi_add_finalizer: collected before delete: true, finalized after delete: 0 + napi_wrap: collected before delete: true, finalized after delete: 1 + napi_add_finalizer: collected before delete: true, finalized after delete: 2 ``` Parent and children (the shape from the report): with the parent created after the children, JSC sweeps the parent's weak handle first, so the parent's queued finalizer ran first and then all 8 child finalizers ran on deleted children (`children finalized after delete: 8`). Created before the children, the children's finalizers ran first and the parent's deletes were plain. The fixture runs both orders. `Bun.gc(true)` is `collectNow(Sync)`, which sweeps synchronously, so the task is queued before `Bun.gc` returns and the same-turn delete is deterministic. A conservative scan can keep an object alive, so each attempt uses a fresh object and the output does not depend on the attempt count. BUN-4NJZ (Windows x64, `bun test`): eight frames inside `index.node`, then `napi_body::Finalizer::run` (`napi_body.rs:2405`, the return address after the callback, which symbolizes as the inlined `napi_internal_remove_finalizer` / `NapiEnv::removeFinalizer` / `BoundFinalizer::BoundFinalizer`), `NapiFinalizerTask::run_on_js_thread`, `dispatch::run_task`. Same shape as BUN-4MXA: the addon's finalizer is what is executing. Drains queued during VM shutdown become cleanup hooks (`NapiFinalizerTask::schedule`); `VirtualMachine::run_cleanup_hooks` repeats while hooks push more, so a chain of drains still runs at exit. The tsfn test on an unfixed ASAN build: ``` ERROR: AddressSanitizer: heap-use-after-free #0 napi_get_threadsafe_function_context src/runtime/napi/napi_body.rs:3290 #1 napitests::tsfn_finalizer_uses_handle standalone_tests.cpp #2 <napi_body::Finalizer>::run napi_body.rs:2403 #3 <NapiFinalizerTask>::run_on_js_thread #4 bun_runtime::dispatch::run_task freed by: ThreadSafeFunction::destroy ``` On a release build the stale read still returns the old context, so that test only fails under ASAN. The two reference tests fail on every build. Experimental modules are unchanged: `callFinalizerFromGC` runs the finalizer during the GC for them, as before. The runtime-owned reference is `NapiRefSelfDeletingWeakHandleOwner` in `napi.cpp`. Other paths checked: - Finalizer deletes its own reference (node-addon-api `ObjectWrap`): `runQueuedFinalizer` takes the ref out of the set before calling, so the delete inside the callback is plain. `test/napi/napi-finalizer-delete-ref.test.ts` covers the experimental variant. - Env cleanup while a finalizer is queued: `wrap_cleanup` runs it at once and clears it. The task later finds the cleared callback and does nothing, so it still runs once. - Task dropped at VM teardown (`has_run_cleanup_hooks`): the entry stays in the set until `~NapiRef` or the env goes away. The set holds the pointer as a key only. - A reused address: set membership means a finalizer is owed, so a drain that reaches a new ref at the same address runs a finalizer that is owed anyway, and the set holds each ref once. - `~NapiRef` does one `ListHashSet::remove`, which returns at once while the env never queued anything. `napi_create_reference` refs never enter the set. - Re-entry from the tsfn finalizer into the function it belongs to: `destroy` holds no borrow and no lock while the callback runs. `napi_release_threadsafe_function` returns `napi_invalid_arg` (`thread_count` is 0) and `napi_unref_threadsafe_function` returns `napi_ok` in both runtimes (asserted by the test). `napi_acquire_threadsafe_function` returns `napi_closing` (`closing` is `Closed`), `napi_call_threadsafe_function` returns `napi_invalid_arg` for the same reason, and `napi_ref_threadsafe_function` is a no-op because `maybe_queue_finalizer` disabled the keepalive. None of them frees the function or schedules it, and `finalizer_fun` was taken, so nothing runs the finalizer twice. Suites run with the debug build: `test/napi/napi.test.ts`, `napi-finalizer-delete-ref.test.ts`, `napi-value-ffi.test.ts`, and the node-napi-tests suites `6_object_wrap`, `7_factory_wrap`, `8_passing_wrapped`, `test_finalizer`, `test_reference`, `test_reference_double_free`, `test_general` (both), `test_instance_data` (both), `test_threadsafe_function`, `test_reference_by_node_api_version`, `test_env_teardown_gc`, `test_worker_terminate_finalization`, `test_buffer`. All pass (a CI-like `--timeout` is needed locally, the default 5 s is too short for the ASAN build). oven-sh#38506 changes `NapiEnv::inGC()`. `callFinalizerFromGC` calls it, so the two compose. Closes oven-sh#41055. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/napi/napi.test.ts <!-- robobun:evidence:end --> --------- Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
springmin
pushed a commit
that referenced
this pull request
Sep 14, 2026
…back closes the session (oven-sh#42647) ### Problem - A server `RST_STREAM` can make the HTTP/2 fetch client read and free a stream twice. A debug ASAN build of main (09bb546) reports `AddressSanitizer: heap-use-after-free` in `ClientSession::handle_data` (`src/http/h2_client/ClientSession.rs:848`, the `s.state != StreamState::Closed` read). The stream was freed by `drop_stream` in `ClientSession::fail_streams` (`ClientSession.rs:943`). A release build has no report, only heap corruption. - It needs a request with a custom TLS context (for example `tls: { serverName }`) whose entry left the 60-entry context cache. That request then holds the last ref. Its terminal callback drops the context, the drop closes the session's socket, and `on_close` runs `fail_streams` inside the deliver loop. ### Fix - While `delivering` is set, `fail_streams` fails each client and leaves the streams in `streams` and `by_http_id`. The deliver loop already removes a stream that has no client, through `remove_stream`, so each stream is freed once. - Outside the loop `fail_streams` behaves as before. - Correct because the loop's tail is safe on a session that is already closed: `maybe_release` returns at the `registry_index` sentinel, and a write to the closed socket writes nothing. - Verified: `test/js/web/fetch/fetch-http2-client.test.ts` (two new ASAN-only tests, 67 pass). With `src/` at main the RST test fails. Also `fetch-http2-leak.test.ts` (7 pass) and `fetch-http2-adversarial.test.ts` (20 pass). ### Background - `ClientSession` is one HTTP/2 connection of the fetch client. `streams` maps a stream id to a heap `Stream`. Each `Stream` points at the `HTTPClient` of one `fetch()`. - `handle_data` parses frames, then delivers each ready stream to its client in a loop, with `delivering` set. A terminal delivery runs the result callback at once, on the HTTP thread. - Custom TLS contexts are refcounted. The cache holds one ref and each request holds one. <details><summary>Notes</summary> This is the part of oven-sh#31788 that main still needs. oven-sh#31788 was closed as stale, and all five of its `src/` files conflict with main now. - Its first defect, `abort_by_http_id` with no ref on the session, is fixed on main: since oven-sh#37870 every entry point goes through `ClientSession::enter`, which holds a `RefPtr`. The abort test from oven-sh#31788 passes on main. It is included here because main has no test that evicts a TLS context. - Its third part, a `ctx` field on `PendingConnect`, has no reproduction and is not included. ASAN report on main, trimmed: ``` ERROR: AddressSanitizer: heap-use-after-free READ of size 1 thread T2 (HTTP Client) #0 <State as PartialEq>::eq src/http/h2_client/Stream.rs:104 #2 ClientSession::handle_data src/http/h2_client/ClientSession.rs:848 #4 ClientSession::enter src/http/h2_client/ClientSession.rs:242 #6 Handler<true>::on_data src/http/HTTPContext.rs:1386 freed by thread T2 (HTTP Client) here: #12 drop_stream src/http/h2_client/ClientSession.rs:211 #13 ClientSession::fail_streams src/http/h2_client/ClientSession.rs:943 ``` The two tests share one helper. The child holds a stream open on a context made by `serverName`, runs 61 more TLS configs to evict it, writes `evicted` to stderr, and then ends the held request: one test aborts it, the other has the server send `RST_STREAM(CANCEL)`. Both check that a later `fetch()` still works. They run only under ASAN and carry a 30 s timeout, because each child makes 62 TLS handshakes and a regression needs time to print its report. Without the longer timeout the failing run on main shows a timeout at 5 s, not the report. The tests are serial on purpose: each one fills the context cache, which would evict the context that a concurrent test depends on. One older test in the same file changed: `concurrent requests multiplex on one h2 session`. Its server held each stream for 100 ms and the test expected 8 open at once. On a loaded machine it saw 7 (2 of 9 runs of the file). The server now answers when the eighth stream is open, so the test uses no timer. After the change the file passed 4 of 4 runs on the same loaded machine. </details> <!-- robobun:evidence:begin --> --- **[human-review]** gate passed · iteration 1 · 2 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 1 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" "test/js/web/fetch/fetch-http2-client.test.ts" bun test v1.4.3 (b993710) test/js/web/fetch/fetch-http2-client.test.ts: (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > GET: status, headers and body round-trip [1309.07ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > response body larger than one DATA frame [1215.67ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > concurrent requests multiplex on one h2 session [1497.63ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST with ReadableStream body streams as raw DATA frames [730.67ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > gzip content-encoding is decompressed [2353.52ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST: request body is delivered as DATA frames [2514.64ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST with ReadableStream body larger than initial send window [927.77 ... (truncated) release without fix: 2 skipped bun test v1.4.3-canary.1 (f92be71) test/js/web/fetch/fetch-http2-client.test.ts: (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > concurrent requests multiplex on one h2 session [110.34ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > gzip content-encoding is decompressed [115.04ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > response trailers are consumed without breaking the body [99.30ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > connection-specific request headers are stripped before HPACK [111.10ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > cold-start: parallel requests coalesce onto one TLS connect [135.66ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > response body larger than one DATA frame [147.00ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > GET: status, headers and body round-trip [160.06ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > keep-alive: sequential requests reuse one h2 session [139.51ms] (pass) fetch() over H ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" "test/js/web/fetch/fetch-http2-client.test.ts" bun test v1.4.3 (b993710) test/js/web/fetch/fetch-http2-client.test.ts: (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > GET: status, headers and body round-trip [1532.08ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > response body larger than one DATA frame [1265.37ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST: request body is delivered as DATA frames [1731.76ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST with ReadableStream body streams as raw DATA frames [501.02ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > gzip content-encoding is decompressed [1802.42ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > concurrent requests multiplex on one h2 session [1743.84ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > concurrent ReadableStream uploads route each chunk to its own stream ... (truncated) release with fix: 2 skipped $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 710ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [0/5] cargo bun_runtime → libbun_runtime.a �[1m�[92m Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) �[1m�[92m Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno) �[1m�[92m Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr) �[1m�[92m Compiling�[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys) �[1m�[92m Compiling�[0m bun_safety v0.0.0 (/workspace/bun/src/safety) �[1m�[92m Compiling�[0m bun_base64 v0.0.0 (/workspace/bun/src/base64) �[1m�[92m Compiling�[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys) �[1m�[92m Compiling�[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys) �[1m�[92m Compiling�[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd) �[1m�[92m Compiling�[0m bun_picohttp v0.0.0 (/workspace/bun/src/picohttp) �[1m�[92m Compiling�[0m bun_brotli v0.0.0 (/workspace/bun/src/brotli) �[1m�[92m Compiling�[0m bun_output v0.0.0 (/workspace/bun/src/output) �[1m�[92m Compiling�[0m bun_clap v0.0.0 (/workspace/bun/src/clap) �[1m�[92m Compiling�[0m bu ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/http/h2_client/ClientSession.rs | 12 ++- test/js/web/fetch/fetch-http2-client.test.ts | 118 ++++++++++++++++++++++++--- 2 files changed, 117 insertions(+), 13 deletions(-) ``` </details> **gate history** · 1 passed · 1 rejected · iteration 1 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/http/h2_client/ClientSession.rs 2 1 24 test/js/web/fetch/fetch-http2-client.test.ts 1 2 24 ``` </details> <!-- robobun:evidence:end -->
springmin
pushed a commit
that referenced
this pull request
Sep 15, 2026
…ide its own socket callback (oven-sh#42693) ### Problem - A `fetch()` with its own TLS context (for example `tls: { serverName }`) holds the last ref to that `HTTPContext` once the 60-entry context cache evicts it. When the server closes the connection, a debug ASAN build reports `AddressSanitizer: heap-use-after-free` in `us_internal_ssl_detach` (`packages/bun-usockets/src/crypto/openssl.c:1827`). A release build shows nothing. - The request drops its ref in the result callback (`src/http/AsyncHTTP.rs:745`), inside the close callback of the context's own socket. That frees the context and the socket group embedded in it. `us_internal_ssl_on_close` then reads `s->group->loop` (`openssl.c:2303`). - Found by fuzzing, no user report. It needs over 60 `tls` configs in one process, or a request older than the 30-minute cache TTL. ### Fix - The last deref of an `HTTPContext` no longer frees it in place. `#[ref_count(destroy = Self::destroy_between_ticks)]` queues it on the HTTP thread, and `process_events` frees it between `drain_events()` and the next `tick()`. - Correct because all work of the HTTP thread runs inside those two calls. No socket callback is on the stack between them, whoever dropped the last ref. - Verified: `test/js/web/fetch/fetch-http2-client.test.ts`. Two new ASAN-only tests (h2 and HTTP/1.1) fail with `src/` at main. A third checks that the context is still freed. ### Background - An `HTTPContext` embeds one uSockets socket group, and each socket points into it (`s->group`). - Custom TLS contexts are refcounted (oven-sh#29334). The cache holds one ref and each request holds one. Eviction drops only the cache ref. - `HttpThread::process_events` is the HTTP thread's loop. `drain_events()` starts and aborts requests. `tick()` runs the socket callbacks. - `WindowsNamedPipeContext` defers its free with the same hook. <details><summary>Notes</summary> ASAN report on main (3f7f046), HTTP/1.1 run, trimmed: ``` ERROR: AddressSanitizer: heap-use-after-free READ of size 8, 72 bytes inside of 208-byte region, thread T4 (HTTP Client) #0 us_internal_ssl_detach packages/bun-usockets/src/crypto/openssl.c:1827 #1 us_internal_ssl_on_close packages/bun-usockets/src/crypto/openssl.c:2303 #2 us_internal_socket_close_raw packages/bun-usockets/src/socket.c:328 #3 us_internal_ssl_on_end packages/bun-usockets/src/crypto/openssl.c:2393 freed by thread T4 (HTTP Client): #12 HTTPContext<true>::destroy #15 RefPtr<HTTPContext<true>>::drop oven-sh#19 AsyncHTTP::on_async_http_callback_raw src/http/AsyncHTTP.rs:745 oven-sh#21 HTTPClient::dispatch_result_and_reset src/http/lib.rs:1618 oven-sh#22 HTTPClient::fail src/http/lib.rs:3970 oven-sh#23 HTTPClient::on_close::<true> src/http/lib.rs:2163 oven-sh#24 Handler<true>::on_close src/http/HTTPContext.rs:1341 oven-sh#28 us_internal_ssl_on_close packages/bun-usockets/src/crypto/openssl.c:2301 ``` The h2 run frees through `fail_from_h2`, called by `ClientSession::fail_streams`, called by `ClientSession::on_close`. oven-sh#42647 fixed a different defect with the same trigger (a stream freed twice in the h2 deliver loop). With this change that trigger is gone too: the context's `Drop`, which closes the session's socket, no longer runs inside the deliver loop. One more path fails on main with the same report: a keep-alive socket that the server closes before it answers. The request dials again from inside the close callback, and the new context displaces the ref on the old one. A redirect, an idle timeout, a DNS failure, an abort and a normal end of the response on an evicted context show no report on main. Why the fix is in `HTTPContext` and not in uSockets. `HTTPContext` is the only socket group owner in bun that freed its group from inside a socket callback. `Listener` frees its group from a finalizer, and the other TLS users share per-VM groups. The safety contract on `SocketGroup::destroy` (`src/uws_sys/SocketGroup.rs`) already says that it must not run while the loop walks the group. A C-side change that passes `loop` into `us_internal_ssl_detach` also stops this report. It leaves the context free to die under Rust frames that still hold its raw pointer, and the next read of `s->group` after a dispatch brings the report back (that read came with oven-sh#31584, and `ssl_release_batch` after it). Two comments in uSockets said the opposite of the Rust contract: `us_socket_group_deinit` called a deinit from `on_close` fine. They now say what `SocketGroup::destroy` says. No C code changes. An earlier draft of this branch routed each holder (`AsyncHTTP::on_async_http_callback_raw`, `HttpThread::connect`) through a queue of refs. The hook replaces that: no holder can get it wrong, and only dead contexts are queued, not one ref per request. No wakeup is needed. A context that dies during `tick()` is freed as soon as that tick returns. One that dies during `drain_events()` (a request that fails at start, a cache eviction) is freed right after it. At process exit the queue is not drained, like the cache. Related history: oven-sh#31660 (closed) is a crash of the same class in the Zig implementation, where the last ref went away inside `onLongTimeout`. Tests: - The ASAN-only block from oven-sh#42647 now takes a protocol. The child holds a response open on a context made by `serverName`, runs 61 more TLS configs to evict it, and writes `evicted` to stderr. The two new tests then destroy the server side of the held TLS socket, over h2 and over HTTP/1.1. With `src/` at main both fail with the report above. - `an evicted custom TLS context is freed when its last request ends` runs on each build. It does not fail on main. It exists because the deferral adds a way to leak: if the queue is never drained, a dead context lives forever. The test keeps one request busy on a context, leaves a second connection idle in that context's keep-alive pool, evicts the context, ends the busy request, and waits for the server to see the idle connection close. With the `heap::destroy` call removed the test times out. Suites run on the debug ASAN build: `fetch-http2-client.test.ts` (70 pass), `fetch-http2-leak.test.ts` (7), `fetch-http2-adversarial.test.ts` (20), `fetch.tls.test.ts` (34), `fetch-redirect.test.ts` (30), `fetch-tls-abortsignal-timeout.test.ts` (6), `fetch-proxy-tls-intern-race.test.ts` (1), `tls-keepalive.test.ts` (4 pass, 2 skip), `fetch-abort-ssl-context-eviction.test.ts` (1), `regression/issue/27358.test.ts` (2). On a release build of this branch: `tls-keepalive.test.ts` 6 pass (same config: 20000 requests, 0 MB growth. 200 distinct configs: 1 MB growth), `fetch-http2-client.test.ts` 66 pass and 4 skip, `fetch-abort-ssl-context-eviction.test.ts` 1 pass. </details> <!-- robobun:evidence:begin --> --- **[human-review]** gate passed · iteration 0 · 5 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 2 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" "test/js/web/fetch/fetch-http2-client.test.ts" bun test v1.4.3 (b993710) test/js/web/fetch/fetch-http2-client.test.ts: (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > GET: status, headers and body round-trip [1172.89ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > response body larger than one DATA frame [989.87ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > concurrent requests multiplex on one h2 session [1222.39ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST with ReadableStream body streams as raw DATA frames [467.33ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST: request body is delivered as DATA frames [1562.88ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > gzip content-encoding is decompressed [1655.77ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST with ReadableStream body larger than initial send window [960.24m ... (truncated) release without fix: 4 skipped bun test v1.4.3-canary.1 (722de17) test/js/web/fetch/fetch-http2-client.test.ts: (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > GET: status, headers and body round-trip [73.34ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > response body larger than one DATA frame [65.19ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > gzip content-encoding is decompressed [89.92ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST: request body is delivered as DATA frames [102.83ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > connection-specific request headers are stripped before HPACK [74.16ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > multiple Set-Cookie response headers survive HPACK decode [74.43ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > concurrent requests multiplex on one h2 session [101.86ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > abort sends RST_STREAM; siblings on the session survive [107.88ms] (pass) fetch() over HTTP/2 (BUN_FE ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" "test/js/web/fetch/fetch-http2-client.test.ts" bun test v1.4.3 (b993710) test/js/web/fetch/fetch-http2-client.test.ts: (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > GET: status, headers and body round-trip [1119.35ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > response body larger than one DATA frame [988.06ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > concurrent requests multiplex on one h2 session [1222.20ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST with ReadableStream body streams as raw DATA frames [531.27ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST: request body is delivered as DATA frames [1925.52ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > gzip content-encoding is decompressed [2051.61ms] (pass) fetch() over HTTP/2 (BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP2_CLIENT) > POST with ReadableStream body larger than initial send window [837.85m ... (truncated) release with fix: 4 skipped $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 846ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/128] gen generated_host_exports.rs generated_host_exports.rs: 122 exports (host=5, lazy=10, generic=107, rust=0); 243 extern-C blocks audited [2/128] gen cpp.rs (cppbind) [3/128] gen JS modules (bundle-modules) Preprocess modules (9358ms) Bundle modules (81ms) Postprocesss modules (248ms) Bundle Functions (727ms) Generate Code (35ms) [10.46s] Bundled "src/js" for production 2600 kb 197 internal modules 13 native modules 50 internal functions across 16 files [3/127] cargo bun_runtime → libbun_runtime.a �[1m�[92m Compiling�[0m bun_threading v0.0.0 (/workspace/bun/src/threading) �[1m�[92m Compiling�[0m bun_react_compiler v0.0.0 (/workspace/bun/src/react_compiler) �[1m�[92m Compiling�[0m bun_io v0.0.0 (/workspace/bun/src/io) �[1m�[92m Compiling�[0m bun_watcher v0.0.0 (/workspace/bun/src/watcher) �[1m�[92m Compiling�[0m bun_crash_handler v0.0.0 (/workspace/bun/src/crash_handler) �[1m�[92m Compiling�[0m bun_event_loop v0.0.0 (/workspace/bun/src/event_loop) �[1m�[92m Compiling�[0m bun_ ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` packages/bun-usockets/src/context.c | 10 ++- packages/bun-usockets/src/loop.c | 10 +-- src/http/HTTPContext.rs | 17 +++- src/http/HTTPThread.rs | 18 ++++ test/js/web/fetch/fetch-http2-client.test.ts | 125 ++++++++++++++++++++++----- 5 files changed, 151 insertions(+), 29 deletions(-) ``` </details> **gate history** · 1 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests packages/bun-usockets/src/context.c 1 1 28 packages/bun-usockets/src/loop.c 1 1 28 src/http/HTTPContext.rs 4 5 28 src/http/HTTPThread.rs 5 15 28 test/js/web/fetch/fetch-http2-client.test.ts 4 11 28 ``` </details> <!-- robobun:evidence:end -->
springmin
pushed a commit
that referenced
this pull request
Sep 15, 2026
…text (oven-sh#42520) ### Problem - `class Upload extends File {}` inside a `node:vm` context crashes on `new Upload(...)`: `panic(main thread): Segmentation fault at address 0x28`. `class S extends fs.Stats {}`, `BigIntStats`, and `Reflect.construct(File, args, fnFromContext)` crash the same way. Node v26.3.0 constructs the object. - These constructors do `static_cast<Zig::GlobalObject*>(getFunctionRealm(lexicalGlobalObject, newTarget))` (`JSDOMFile.cpp:75`, `NodeFSStatBinding.cpp:843`, `NodeFSStatFSBinding.cpp:372`). For a function from a context the realm is a `NodeVMGlobalObject`, which is not a `Zig::GlobalObject`. `JSBlobStructure()` and `getStructure<isBigInt>()` then read a `LazyClassStructure` past the end of the cell. ### Fix - The three sites call `defaultGlobalObject(getFunctionRealm(...))`. It returns the realm when it is a Bun global, and the default Bun global when it is not. - The other native constructors already do this (`generate-classes.ts:2387`, `JSDOMWrapperCache.cpp:42`, `NodeDirent.cpp:177`). - The `StatFs` constructor is not reachable from JS, so that site has no test. - Verified: `test/js/node/vm/vm.test.ts`, four new cases. All four exit with SIGSEGV on 1.4.3-canary.1. Also ran the `fs.Stats`, `Blob`, and globals suites. ### Background - `newTarget` is the constructor that `new` was applied to. In `new Upload()`, `File` runs with `newTarget === Upload`. - `getFunctionRealm(newTarget)` returns the global object the function was created in. A native constructor takes its base `Structure` from that global. `createSubclassStructure` then applies `newTarget.prototype`. - A `node:vm` context has its own global, `NodeVMGlobalObject`. It and `Zig::GlobalObject` (the Bun global) are sibling subclasses of `Bun::GlobalScope`. Only `Zig::GlobalObject` holds the structures of Bun classes. - A ShadowRealm global is a `Zig::GlobalObject`. The comment at these sites names that case, and the cast was valid for it. <details><summary>Notes</summary> Repro (`bun f.js file`, `bun f.js stats`, `bun f.js bigint`): ```js const vm = require("vm"), fs = require("fs"); const ctx = vm.createContext({ File, Stats: fs.Stats }); const which = process.argv[2] || "file"; try { if (which === "file") console.log("ok", vm.runInContext(`class Upload extends File {}; new Upload(["a"], "a.txt")`, ctx).name); if (which === "stats") console.log("ok", typeof vm.runInContext(`class S extends Stats {}; new S()`, ctx).isFile); if (which === "bigint") console.log("ok", typeof Reflect.construct(fs.statSync(".", { bigint: true }).constructor, [], vm.runInContext("(function(){})", ctx))); } catch (e) { console.log("threw", e.message); } console.log("survived"); ``` | | `file` | `stats` | `bigint` | | --- | --- | --- | --- | | 1.4.3-canary.1 | segfault at 0x28 | segfault at 0x28 | segfault at 0x28 | | this branch | `ok a.txt` | `ok function` | `threw Invalid argument type in ToBigInt operation` | | node v26.3.0 | `ok a.txt` | `ok function` | `threw Cannot mix BigInt and other types` | ASAN on main with `Malloc=1` in the environment. `heap-buffer-overflow`, `READ of size 8`, "located 3776 bytes after 4184-byte region" (`File`), 896 bytes after (`Stats`), 912 bytes after (`BigIntStats`). The region is the `NodeVMGlobalObject` cell. ``` #0 JSC::LazyProperty<JSC::JSGlobalObject, JSC::Structure>::getInitializedOnMainThread LazyProperty.h:95 #1 JSC::LazyClassStructure::getInitializedOnMainThread LazyClassStructure.h:104 #2 Zig::GlobalObject::JSBlobStructure() ZigGeneratedClasses+lazyStructureHeader.h:7 #3 JSDOMFile::construct src/jsc/bindings/JSDOMFile.cpp:79 #5 llint_op_super_construct_varargs ``` ``` #1 JSC::LazyClassStructure::getInitializedOnMainThread LazyClassStructure.h:104 #2 Bun::getStructure<false>(Zig::GlobalObject*) src/jsc/bindings/NodeFSStatBinding.cpp:167 (<true>: line 164) #3 Bun::constructJSStatsObject<false>(JSGlobalObject*, CallFrame*) src/jsc/bindings/NodeFSStatBinding.cpp:847 #4 Bun::constructStats src/jsc/bindings/NodeFSStatBinding.cpp:915 ``` Without `Malloc=1` the debug build of main reads zero there and stops at `Structure.h:415: runtime error: member call on null pointer of type 'const JSC::Structure *'`. Fail-before and pass-after, same test file: - `USE_SYSTEM_BUN=1 bun test test/js/node/vm/vm.test.ts -t "belongs to a context"`: 0 pass, 4 fail (`exitCode: 139`, `signalCode: "SIGSEGV"`). - Debug ASAN build of main with only the test added: 0 pass, 4 fail (`exitCode: 1`). - Debug ASAN build of this branch: 4 pass. The whole file: 285 pass, 0 fail. Other cases checked by hand on this branch: - A newTarget that is a plain function, a bound function, or a Proxy from the context. The new test covers these. A bound function has no `prototype`, so the object gets the prototype of the constructor. - A revoked Proxy from the context throws `TypeError: Cannot get function realm from revoked Proxy`. - A newTarget whose `prototype` is not an object falls back to `File.prototype` or `fs.Stats.prototype`. - `class X extends File {}` inside a `ShadowRealm` still works. `File` and `X` both belong to the ShadowRealm global there. - An instance of the context subclass works with `FormData`, `.text()`, and a forced GC. Suites run on the debug build: `test/js/node/vm/vm.test.ts`, `test/js/node/fs/fs-stats-constructor.test.ts`, `test/js/node/fs/fs-stats-truncate.test.ts`, `test/js/node/fs/fs.test.ts -t Stats`, `test/js/bun/globals.test.js`, `test/js/web/fetch/blob.test.ts`, and the Node tests `test-fs-stat.js`, `test-fs-stat-bigint.js`, `test-fs-statfs.js`, `test-fs-watchfile.js`. Related: - oven-sh#32434 changes the `File` cast to `defaultGlobalObject` as one line of a larger `File.prototype` change. It does not touch `fs.Stats`. - oven-sh#42514 fixes the same kind of cast at another site (`UtilInspect.cpp`). - `new BigIntStats(...)` stores `atimeMs` as a Number where Node stores a BigInt, so `.atime` throws. That is a separate bug and is not part of this PR. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/vm/vm.test.ts <!-- robobun:evidence:end -->
springmin
pushed a commit
that referenced
this pull request
Sep 17, 2026
oven-sh#42900) ### Problem - An HTTP/3 `fetch()` whose QUIC connection dies before the response header can abort the process. ASan: `heap-use-after-free READ of size 8` in `HTTPClient::fail_from_h2` (`src/http/lib.rs:2108`), from `ClientSession::retry_or_fail` (`src/http/h3_client/ClientSession.rs:288`). Release builds panic: `fetch on the HTTP thread holds a ticket`. - The retry queues the request on a new session through `ClientContext::connect`. When no connection opens, connect fails that session with `PendingConnect::fail_session`, which fails every request queued on it. That dispatch frees the `AsyncHTTP` the client is part of. Then the retry fails the same client again. ### Fix - `connect` takes the request back off the session before it fails that session. A `false` return leaves the request on no session, so the caller is its only failure path, which is what the other two callers assume. - Correct because the session is one call old: the request `enqueue` just queued is its only entry, so `detach` leaves `fail_session` nothing to fail. The teardown, the registry removal and the session's last reference do not change. - The retried request keeps the error of the stream that closed. `start_` still reports `ConnectionRefused` for its own failed connect. - Verified: `test/js/web/fetch/fetch-http3-client.test.ts`, one new test (main aborts with an empty stdout). Also the three other `fetch-http3-*` suites, `serve-http3` and `serve-protocols`. ### Background - The h3 fetch client pools one QUIC connection per origin. `retry_or_fail` re-sends a stream that closed before any response header, once, on a fresh connection. - `ClientContext::connect` finds a pooled connection or opens one, and queues the request. `enqueue` binds a `Stream` to the request before the QUIC connect, because that stream has to exist when the handshake completes. - `HTTPClient::start_` sets `defer_terminal_dispatch_until_connecting_is_complete` before its own connect call, so a failure inside that frame is recorded and dispatched later. That flag is why the two initial connect sites survived the double failure. <details><summary>Notes</summary> **Fail-before.** With `src/` and `packages/` back on `55c11065f2`, the new test gives `exitCode: 1` and an empty stdout. That run, the passing run and the suites above were on `55c11065f2` plus this change, built with LLVM 21. The branch has since merged main, which needs LLVM 23 (oven-sh#42851). The build environment used here does not have it, so on the merged tree only `cargo check` and `cargo clippy` for `bun_http` were run locally, and CI is the test run for it. The three commits that merge brought in touch none of the files involved. The ASan frames are the report above: ``` READ of size 8 at 0x... thread T4 (HTTP Client) #2 <bun_http::HTTPClient>::fail_from_h2 src/http/lib.rs:2108 #3 <ClientSession>::retry_or_fail src/http/h3_client/ClientSession.rs:288 #4 h3_client::callbacks::on_conn_close src/http/h3_client/callbacks.rs:151 freed by thread T4 (HTTP Client) here: #7 <AsyncHTTP>::on_async_http_callback_raw src/http/AsyncHTTP.rs:783 #10 <bun_http::HTTPClient>::fail_from_h2 src/http/lib.rs:2122 #11 <PendingConnect>::fail_session src/http/h3_client/PendingConnect.rs:149 #12 <ClientContext>::connect src/http/h3_client/ClientContext.rs:179 #13 <ClientSession>::retry_or_fail src/http/h3_client/ClientSession.rs:287 ``` A release build aborts as well, so the fault is not an ASan artifact: `on_async_http_callback_raw` resets the client's stage before the dealloc, so the once-only guard in `fail_from_h2` cannot stop the second dispatch. Making that guard survive the reset is a separate change. **How the test reaches it.** A connect to a resolved hostname probes each address with a throwaway UDP `connect(2)`, and gives up when no entry is reachable (`packages/bun-usockets/src/quic.c`, `us_quic_connect_result`). An `LD_PRELOAD` shim allows the first probe and refuses every later one, so the reconnect fails inside `connect`. `rejectUnauthorized` against the suite's self-signed certificate fails the handshake, which is what closes the stream before any header and starts the retry. `localhost` answers from `is_localhost_name` as `[::1, 127.0.0.1]` without the resolver, so no connect waits for DNS, and the shim refuses the IPv6 entry the way a host without an IPv6 route does, which pins both connects to the same address. Linux only, and only where a C compiler exists, like the DPLPMTUD shim test in `fetch-http3-syscall-fault.test.ts`. 5 runs, 5 passes, about 500 ms each on the debug ASan build. **Other ways to reach the same failure.** Any synchronous failure of the QUIC connect does it: a cached resolver error, an IP literal whose family the shared client endpoint cannot serve, `lsquic_engine_connect` returning NULL, or the shared client UDP endpoint dying on a hard `recvmsg` error and the poll registration for its replacement failing. The last one needs no resolver, so it reaches this path for an IP-literal origin too. One test is enough: all of them end in the same `return false`, and the endpoint-replacement route needs several iterations of a loop to line up. **Earlier shape.** The first version of this PR removed the retry's failure call instead, and documented `connect` as owning the request. Review pushed back: it left both `if !connect { self.fail(..) }` arms in `start_` dead, it made the bool unusable by every caller, and it set the opposite contract from oven-sh#40385, which removes the same double failure from the callee side. This version fixes the callee, which also keeps the closed stream's error in the rejection instead of replacing it with `ECONNREFUSED`. **Scope.** `retry_or_fail` is also edited by oven-sh#41564 (a retry budget) and oven-sh#42579 (no replay of a non-idempotent request), and oven-sh#40598 changes which pre-header closes retry. None of them touch this branch, so this applies on top of any of them, and oven-sh#40385 keeps the same contract. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/web/fetch/fetch-http3-client.test.ts <!-- robobun:evidence:end -->
springmin
pushed a commit
that referenced
this pull request
Sep 30, 2026
### Problem - On Windows, a worker can use the stdin `FileSink` of a `Bun.spawn` child after it is freed. It needs `stdin: "pipe"`, a live child, and a script that never read `proc.stdin`. Debug build: `panic: misaligned pointer dereference: address must be a multiple of 0x8 but is 0xdfdfdfdfdfdf`. A release build reads freed memory silently. - `FileSink::on_close` (`src/runtime/webcore/FileSink.rs:528`) calls `source.close()`. That reaches `Writable::on_close` (`src/runtime/api/bun/subprocess/Writable.rs:114`), which drops the only ref on the sink. `on_close` then continues on freed memory. ### Fix - `FileSink::on_close` holds a ref on the sink until it returns. `on_write` and `run_pending` already use this guard. - A testing hook, `subprocessInternals.closeStdinWriter(proc)`, does the same close as the Windows stop phase, on every platform. - Verified with the hook, in `test/js/bun/util/filesink.test.ts`. Without the guard, the Linux ASAN build reports `heap-use-after-free` in `settle_stream_done`, and a Windows debug build panics. With the guard both pass. The worker test in `test/js/web/workers/worker-terminate-lifetime.test.ts` can fail only on a Windows debug build. - Self-reviewed: 3 concerns raised, 2 addressed (see Notes). ### Background - A `FileSink` is a refcounted writer. For `stdin: "pipe"` the Subprocess holds one ref. The `proc.stdin` getter moves it to a JS wrapper. `source.close()` tells that owner that the sink closed, and the owner drops its ref. - The stop phase is the first step of worker teardown (oven-sh#37075). On Windows it closes each libuv pipe of the worker through its writer, which calls `FileSink::on_close`. ### Downsides - None found for the guard. `on_close` never runs while the sink is destroyed, so the guard never takes a ref from zero. - Still open: the stream-fed stdin leak in the Notes. <details><summary>Notes</summary> **Repro** (windows-x64, debug build, 5 of 5 runs on main at b7ea95a): ```js import { Worker } from "node:worker_threads"; const w = new Worker(` const { parentPort } = require("node:worker_threads"); globalThis.keep = Bun.spawn({ cmd: [process.execPath, "-e", "setTimeout(() => {}, 4000)"], stdin: "pipe", stdout: "ignore", stderr: "ignore" }); setTimeout(() => parentPort.postMessage("go"), 300); `, { eval: true }); await new Promise(r => w.once("message", r)); await w.terminate(); console.log("survived"); ``` **Symbolized stack** (llvm-symbolizer with the PDB, ASLR off, line numbers at b7ea95a): ``` bun_jsc::strong::Impl::get src/jsc/Strong.rs:179 bun_jsc::strong::Optional::get src/jsc/Strong.rs:92 webcore::streams::PipeCell::take_done src/runtime/webcore/streams.rs:908 FileSink::settle_stream_done src/runtime/webcore/FileSink.rs:552 FileSink::on_close src/runtime/webcore/FileSink.rs:530 <FileSink as WindowsStreamingWriterParent>::on_close src/io/PipeWriter.rs:2734 WindowsStreamingWriter<FileSink>::on_close_source src/io/PipeWriter.rs:2010 WindowsStreamingWriter<FileSink>::close src/io/PipeWriter.rs:1212 WindowsStreamingWriter<FileSink>::stop_for_vm_teardown src/io/PipeWriter.rs:1130 open_handles::stop_all_for_vm_teardown src/libuv_sys/open_handles.rs:172 VirtualMachine::stop_phase_sweep src/jsc/VirtualMachine.rs:2554 VirtualMachine::teardown src/jsc/VirtualMachine.rs:2401 WebWorker::shutdown / spin / thread_main src/jsc/web_worker.rs ``` The address matches. `Strong::get` masks the handle to 48 bits, and `0xdfdfdfdfdfdfdfdf & ((1 << 48) - 1)` is `0xdfdfdfdfdfdf`. `0xdf` is the fill that a debug mimalloc writes into a freed block. **Linux ASAN report** (debug ASAN build with the hook and without the guard, from the new test in `filesink.test.ts`): ``` ERROR: AddressSanitizer: heap-use-after-free ... READ of size 8 #0 bun_jsc::strong::Optional::get src/jsc/Strong.rs:91 #1 webcore::streams::PipeCell::take_done src/runtime/webcore/streams.rs:908 #2 FileSink::settle_stream_done src/runtime/webcore/FileSink.rs #3 FileSink::on_close src/runtime/webcore/FileSink.rs #5 PosixStreamingWriter<FileSink>::close src/io/PipeWriter.rs:1048 #7 PollOrFd::close_impl src/io/pipes.rs:118 #10 testing_apis::close_stdin_writer src/runtime/api/bun/subprocess.rs freed by thread T0 here: #12 <FileSink as CellRefCounted>::destroy src/ptr/ref_count.rs:448 #15 <RefPtr<FileSink> as Drop>::drop src/ptr/ref_count.rs:522 oven-sh#19 JsCell<Writable>::set src/ptr/js_cell.rs:94 oven-sh#20 Writable::on_close src/runtime/api/bun/subprocess/Writable.rs:114 oven-sh#22 SourceHandle::close src/runtime/webcore/streams.rs:1043 oven-sh#23 FileSink::on_close src/runtime/webcore/FileSink.rs ``` The use and the free are in the same `FileSink::on_close` call. For this proof I removed only the guard line and kept the hook. With the `src/` of main the new `filesink.test.ts` test also fails, but for a weaker reason: the hook does not exist there. **Why only this path.** Every other path that closes this sink keeps it alive or detaches the source first: - `Subprocess::on_process_exit` clears `source` before `on_attached_process_exit`, which also holds a ref. - `Writable::finalize` and `on_close_io` clear `source` before they drop the ref. - The `proc.stdin` getter takes the pipe out of the `stdin` slot and clears `source`, so `Writable::on_close` is not reached. - POSIX has no stop-phase close of the pipe. The sink dies in `Subprocess::finalize`. A poll HUP with an empty buffer reports `Drained`, not a close. **Variants** (windows-x64 debug, before and after the fix): | worker ends by | stdin | before | after | | --- | --- | --- | --- | | `terminate()` | `"pipe"`, never read | panic | ok | | `process.exit()` in the worker | `"pipe"`, never read | panic | ok | | loop drains (`proc.unref()`) | `"pipe"`, never read | panic | ok | | `terminate()`, `stdout: "pipe"` too | `"pipe"`, never read | panic | ok | | `terminate()` | `"pipe"`, `proc.stdin.write()` pending | ok | ok | | no worker, `closeStdinWriter(proc)` hook | `"pipe"`, never read | panic | ok | After the fix, `fileSinkInternals.liveCount()` is back at the baseline after the `exit` event of the worker in each row. **Suites run with the fix.** - windows-x64 debug: `worker-terminate-lifetime.test.ts` (24 pass, 2 skip), `worker_destruction.test.ts` (5 pass), `spawn.test.ts` (126 pass), `spawn-stdin-readable-stream.test.ts` (36 pass), `spawn-stdin-destroy.test.ts`, `spawn-stdin-pipe-fd-leak.test.ts`, `child_process.test.ts` (53 pass), `filesink.test.ts` (33 pass, 1 fail, see below). The hook test passes 5 of 5 runs and the worker test 3 of 3. - linux-x64 debug ASAN: both new tests, `filesink.test.ts` (70 pass), `spawn-stdin-readable-stream.test.ts` (37 pass), `spawn-stdin-destroy.test.ts`, `worker_destruction.test.ts` (5 pass with `--timeout 120000`). **Self-review.** - Addressed: no CI lane could fail the worker test. The Windows lanes run release builds, where the stale reads do not crash, and Linux does not reach the path. The hook test now fails on an ASAN build of any platform without the guard. The worker test stays, because it is the real trigger, and its comment says which build can fail it. - Addressed: the comment before `clear_keep_alive_ref` said that call can free the sink. With the guard it cannot, so the sentence is gone. - Not changed: the sink keeps its stale `source` after the owner is told. Nothing reads it again on this path, because a later write needs the JS wrapper, and the getter that creates the wrapper clears `source`. - Checked and fine: no caller of `on_close` touches the writer or the sink after the call returns (`on_close_source`, the tail of `close`, `stop_all_for_vm_teardown`, and `PollOrFd::close` on POSIX, where the callback is last). Both writer `Drop` impls close without a report, so `on_close` never runs while the sink is destroyed. **Not in this PR.** On Windows, a worker that ends while `Bun.spawn({ stdin: new ReadableStream({ pull(c) { c.enqueue(new Uint8Array(1024)); return new Promise(() => {}); } }) })` still pumps leaves one `FileSink` alive. `liveCount()` stays at +1, before and after this change. Linux returns to the baseline. The cause is different: the ref that `assign_to_js_stream` takes for the reactions of the pump promise is not released, because the reactions never run after script is forbidden. oven-sh#43729 adds a flag that tracks that ref. **Local failures that this diff does not cause.** - windows-x64 (Server 2019): `test/js/bun/util/filesink.test.ts` "Bun.spawn stdin pipe with an unref'd child" fails 3 of 3 runs with the `src/` of main, with this fix, and with the release canary. - linux-x64 debug ASAN in my container: "terminate() while dns.lookup() is in flight" in the same test file reports a 16 byte LeakSanitizer leak from `node_fs_binding::Binding::new`. No `FileSink` is involved. </details> <!-- robobun:evidence:begin --> --- **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/web/workers/worker-terminate-lifetime.test.ts, test/js/bun/util/filesink.test.ts <!-- robobun:evidence:end -->
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
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.
What does this PR do?
Updates SQLite to version 3.53.100
Compare: https://sqlite.org/src/vdiff?from=3.53.0&to=3.53.100
Auto-updated by this workflow