Skip to content

fix(ohos): add EPERM fallback to all linkat/symlinkat retry paths + extract copy_file_fallback helper - #10

Merged
springmin merged 2 commits into
springmin:ohos-aarch64from
DongZhidong1993:ohos-aarch64-build-20260611-v2
Jun 11, 2026
Merged

springmin merged 2 commits into
springmin:ohos-aarch64from
DongZhidong1993:ohos-aarch64-build-20260611-v2

Conversation

@DongZhidong1993

Copy link
Copy Markdown

fix(ohos): add EPERM fallback to all linkat/symlinkat retry paths + extract copy_file_fallback helper

@autofix-troubleshooter

Copy link
Copy Markdown

Hi! I'm the autofix logoautofix.ci troubleshooter bot.

It looks like you correctly set up a CI job that uses the autofix.ci GitHub Action, but the autofix.ci GitHub App has not been installed for this repository. This means that autofix.ci unfortunately does not have the permissions to fix this pull request. If you are the repository owner, please install the app and then restart the CI workflow! 😃

@springmin
springmin merged commit bfc09f2 into springmin:ohos-aarch64 Jun 11, 2026
2 of 4 checks passed
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
…sweep (oven-sh#32729)

### Crash

```
ASSERTION FAILED: vm().currentThreadIsHoldingAPILock() => vm().heap.mutatorState() != MutatorState::Sweeping
vendor/WebKit/Source/JavaScriptCore/runtime/JSCell.cpp(179) : bool JSC::JSCell::validateIsNotSweeping() const
```

Backtrace (from a release-asan build with asserts):

```
#3  JSC::JSCell::validateIsNotSweeping()
#4  JSC::JSCell::classInfo() const
#5  WTF::uncheckedDowncast<WebCore::JSResumableFetchSink>(JSValue const&)
#6  ResumableFetchSinkPrototype__ondrainSetCachedValue
#7  bun_runtime::webcore::fetch::fetch_tasklet::FetchTasklet::ignore_remaining_response_body
#8  JSC::WeakBlock::sweep()          <- inside GC sweep (Weak finalizer)
#9  JSC::WeakSet::sweep()
#10 JSC::PreciseAllocation::sweep()
#12 JSC::Heap::finalize()
oven-sh#21 JSC::LocalAllocator::allocateSlowCase
oven-sh#23 JSC::ErrorInstance::create        <- ordinary allocation kicked off GC
```

Found by the syscall fault-injection fuzzer's client-side grammar
scenario (fetch/node:http with abort + transient errno on the client
socket). Reproduces ~4/5 under `BUN_JSC_collectContinuously=1`.

### Cause

`FetchTasklet::on_response_finalize` is the
`WeakRefOwner<FetchResponse>::finalize` callback and runs inside
`WeakBlock::sweep` while `MutatorState == Sweeping`. When the response
body is `Locked` without a pending promise or stream it calls
`ignore_remaining_response_body()`, which called:

- `ResumableSink::detach_js()`: writes the sink wrapper's cached
`ondrain` / `oncancel` / `stream` slots via the generated
`ResumableFetchSinkPrototype__*SetCachedValue` helpers. Each does
`uncheckedDowncast<JSResumableFetchSink>(thisValue)`, which reaches
`JSCell::classInfo()` and then issues a write barrier on the wrapper
cell.
- `clear_stream_handlers()`: reaches `ReadableStreamTag__tagged` ->
`object->inherits<JSReadableStream>()` (guarded today, but one boolean
away).

Calling `classInfo()` on any cell while the mutator is sweeping is
forbidden: the cell's `Structure` may already have been swept. Assert
builds catch it; release builds corrupt the heap.

### Fix

Thread a `from_finalizer` flag through `ignore_remaining_response_body`.
When `true` (the `on_response_finalize` caller) skip `detach_js()` and
`clear_stream_handlers()`; only native state is touched. The sink's
JS-side detach still happens from `clear_sink()` in
`FetchTasklet::deinit()`, which runs as an event-loop `ConcurrentTask`
outside any sweep, so nothing leaks.

The `on_stream_cancelled_callback` caller (reader `.cancel()`, runs from
JS on the event loop) passes `false` and keeps the immediate detach.

Also corrects the `ResumableSink::detach_js` doc comment that claimed
finalizer safety.

### Verification

New test at `test/js/web/fetch/fetch-response-finalizer-sweep.test.ts`:
a child process under `BUN_JSC_collectContinuously=1` does 12 iterations
of `fetch()` with a user-constructed `ReadableStream` body (so the sink
takes the JS route with a Strong `js_this`) against a raw TCP server
that sends headers + a partial chunked body and never terminates it,
then drops the `Response` unconsumed and runs `Bun.gc(true)`.

Without the fix (`bun bd`, src/ stashed):

```
exitCode: 134
stderr: ASSERTION FAILED: vm().currentThreadIsHoldingAPILock() => vm().heap.mutatorState() != MutatorState::Sweeping
```

With the fix: `stdout: "ok"`, `exitCode: 0`.

`test/js/web/fetch/fetch-backpressure.test.ts` (exercises the
`on_stream_cancelled_callback` path) passes unchanged.

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
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 15, 2026
…ry rewrite (oven-sh#34271)

`test/js/bun/util/filesystem_router.test.ts` went red on alpine x64 in
build [73276](https://buildkite.com/bun/bun/builds/73276): the `reload()
while Bun.build() resolves the same directory` subprocess segfaulted in
`bust_dir_cache_recursive`, inlined from `NonNull::new`.

## Cause

`RealFS::entries_at` (`src/resolver/lib.rs`) replaces a cached
`DirEntry` in place when the caller's resolver generation is newer than
the cached listing's. The replacement at `*e_ptr = new_entry` drops the
old `DirEntry`, which drops its `data: StringHashMap<*mut Entry>` and
frees the hashmap's bucket allocation. The function's comment says
`entries_mutex held by caller`, but that is only true on one of the five
paths that reach it: `dir_info_uncached`, when entered from
`dir_info_cached_miss`. The other callers (`finalize_result`,
`handle_esm_resolution`, `load_index_with_extension`,
`Transpiler::run_env_loader`) all reach `entries_at` after
`dir_info_cached_maybe_log` has already returned and released both
`RESOLVER_MUTEX` and `entries_mutex`.

`FileSystemRouter::reload()` and `RouteLoader::load` iterate the same
`DirEntry.data` map under `entries_mutex` (the snapshot pattern oven-sh#33056
introduced for exactly this kind of concurrent rewrite). With
`entries_at`'s rewrite unsynchronized, a `Bun.build()` on the bundler
thread can drop the map while `reload()` on the JS thread is
mid-iteration.

The generation mismatch is what makes `entries_at` enter its rewrite
branch, so the window only opens once the bundle thread has processed at
least one batch (it bumps its own generation after every queue drain);
every subsequent `Bun.build()` then re-reads any directory that
`reload()` just refreshed to generation 0.

ASAN catches it as a heap-use-after-free with the two sides of the race
laid out exactly:

```
READ of size 16 (thread T0):
  #6 HashMap::values
  #7 StringHashMap<*mut Entry>::values                       src/collections/array_hash_map.rs:1864
  #8 FileSystemRouter::bust_dir_cache_recursive              src/runtime/api/filesystem_router.rs:395
  #9 FileSystemRouter::bust_dir_cache                        src/runtime/api/filesystem_router.rs:451
  #10 FileSystemRouter::reload                               src/runtime/api/filesystem_router.rs:476

freed by thread T11 (Bundler):
  #11 drop_in_place<bun_resolver::fs_full::DirEntry>
  #12 bun_resolver::fs::RealFS::entries_at                   src/resolver/lib.rs:1639
  #13 DirInfo::get_entries_ref                               src/resolver/dir_info.rs:266
  #14 Resolver::finalize_result                              src/resolver/resolver.rs:1714
  #15 Resolver::resolve_and_auto_install                     src/resolver/resolver.rs:1485
  ...
  oven-sh#23 BundleThread::generate_in_new_thread                   src/bundler/BundleThread.rs:276

previously allocated by thread T0:
  oven-sh#17 HashMap::reserve
  oven-sh#18 Resolver::dir_info_cached_miss                         src/resolver/resolver.rs:4591
  oven-sh#19 Resolver::dir_info_cached_maybe_log                    src/resolver/resolver.rs:4201
  oven-sh#20 Resolver::read_dir_info                                src/resolver/resolver.rs:4118
  oven-sh#21 FileSystemRouter::reload                               src/runtime/api/filesystem_router.rs:492
```

(The use side is sometimes `RouteLoader::load` at
`src/router/lib.rs:816` instead; same map, same lock.)

This has been the shape of `entries_at` since the Rust port; oven-sh#33056
narrowed the race by snapshotting under the lock but assumed the rewrite
side already held it.

## Fix

`entries_at` now takes `entries_mutex` itself, matching
`read_directory_with_iterator` which already does. The one call path
that reaches it with the lock already held (`dir_info_cached_miss` ->
`dir_info_uncached` -> `parent_.get_entries_ref`) routes through a new
`entries_at_locked` / `get_entries_ref_locked` pair so the non-recursive
mutex is not re-entered. That path is the only one that passes a
non-`None` parent to `dir_info_uncached`; the other caller
(`dir_info_for_resolution`) passes `None`, so the parent branch
containing the accessor never runs there.

## Test

The existing concurrency test now awaits one `Bun.build()` first, so the
bundle thread's generation is already past zero when the concurrent
rounds start, and then runs forty reload/build rounds instead of one.
That is the shape that reaches the stale-generation rewrite at all; the
original single-round fixture usually completes with every build still
on generation 0.

The race is scheduling-dependent. Pinning the fixture to a single core
reproduces the ASAN use-after-free on roughly 3 in 10 runs against an
unpatched debug build and 0 in 15 with this change; with all 16 cores
available the unpatched build reproduces at roughly 1 in 30. The
assertions are otherwise the same as before, so the test continues to
cover the behavior oven-sh#33056 added.

Also ran the full `filesystem_router.test.ts`,
`test/bundler/bun-build-api.test.ts` (including the thousands-of-builds
test that exercises the generation path heavily),
`test/js/bun/resolve/resolve.test.ts`, `test/cli/hot/hot.test.ts`,
`test/cli/watch/watch.test.ts`, `test/bake/framework-router.test.ts`,
and `bun run rust:check-all` (10/10 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/bun/util/filesystem_router.test.ts

<!-- robobun:evidence:end -->
springmin pushed a commit that referenced this pull request Jul 16, 2026
…e drain (oven-sh#34278)

## Problem

`test/js/node/test/parallel/test-worker-stdio-flush.js` went red on the
`debian 13 x64-asan` lane of [build
73374](https://buildkite.com/bun/bun/builds/73374) with:

```
==18202==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 32 byte(s) in 1 object(s) allocated from:
    #9  ConcurrentTask::new src/event_loop/ConcurrentTask.rs:305
    #10 ConcurrentTask::create src/event_loop/ConcurrentTask.rs:319
    #12 bun_jsc::virtual_machine_exports::queue_task_concurrently src/jsc/virtual_machine_exports.rs:140
    #13 ScriptExecutionContext::postTaskConcurrently src/jsc/bindings/ScriptExecutionContext.cpp:266
    #14 ScriptExecutionContext::postTaskTo src/jsc/bindings/ScriptExecutionContext.cpp:125
    #15 MessagePortPipe::scheduleDrain src/jsc/bindings/webcore/MessagePortPipe.cpp:74
    oven-sh#16 MessagePort::postMessage src/jsc/bindings/webcore/MessagePort.cpp:143
```

The leaked allocation is a `ConcurrentTask` (and the `EventLoopTask` it
wraps) left in an exiting worker's `concurrent_tasks` queue after the
queue has been drained for the last time.

## Cause

`WebWorker::shutdown()` runs `process.on('exit')` handlers, then drains
the worker's concurrent queue via `release_queued_tasks_for_shutdown()`,
then enters `WebWorker__teardownJSCVM` which (first thing) calls
`ctx->markTerminating()`. `ScriptExecutionContext::postTaskTo` already
refuses to enqueue onto a terminating context, but between the drain and
the flag flip there is a short window where a cross-thread poster still
sees `isTerminating() == false` and enqueues.

In the failing test the worker writes to `process.stdout` inside its
`exit` handler. The parent's captured-stdout reader acks each chunk with
`port.postMessage(true)` (`src/js/node/worker_threads.ts`
`makePortReadable._read`), which routes through
`MessagePortPipe::scheduleDrain` to `postTaskTo(workerCtxId, ...)`. When
the ack lands in that window it is pushed onto the worker's
`concurrent_tasks`; nothing drains it again, and the worker's VM box is
`dealloc`'d raw, so LSan reports the `ConcurrentTask` as a direct leak.

The window is a few assignments plus one FFI call wide, so it hits
probabilistically; the `release-asan` build is fast enough to line up
occasionally, debug essentially never.

The ordering was introduced in oven-sh#31216; oven-sh#29917 described the same gap ("a
task posted between this drain and `removeFromContextsMap()` inside
`teardownJSCVM` still leaks") but left it open.

## Fix

- `ScriptExecutionContext::markTerminating()` now takes
`allScriptExecutionContextsMapLock`, the same lock `postTaskTo` holds
across its `isTerminating()` check and `postTaskConcurrently()` enqueue.
That makes the flag flip a proper fence against concurrent posters: any
`postTaskTo` critical section either runs entirely before
`markTerminating()` (its task is visible to the subsequent drain) or
entirely after (it observes `true` and drops).
- `WebWorker::shutdown()` calls the new `extern "C"
ScriptExecutionContext__markTerminating` immediately before
`release_queued_tasks_for_shutdown()`, closing the window. The later
`markTerminating()` inside `WebWorker__teardownJSCVM` is now redundant
but harmless.

No behaviour change for `process.on('exit')` itself: that runs before
the new call, so a parent ack posted while the handler is running is
still enqueued and then freed by the drain (never executed, same as
before). Only posts that would have landed after the drain are now
dropped instead of leaked.

## Verification

The gap is too narrow to reproduce unassisted against a debug build: 200
iterations of the Node test with the CI LSan env, and 150 worker
shutdowns with 64 Atomics-synchronized MessagePorts each, all pass on an
unpatched `bun bd`. Widening the gap with a temporary
`std::thread::sleep(5ms)` between `release_queued_tasks_for_shutdown()`
and `WebWorker__teardownJSCVM` makes it deterministic:

| build | `test-worker-stdio-flush.js` under LSan | 200-port
Atomics-synchronized probe |
| --- | --- | --- |
| unpatched + 5 ms sleep | 5/5 leak (`32 byte(s) ConcurrentTask`) | 5/5
leak |
| this PR + 5 ms sleep | 10/10 clean | 5/5 clean |
| this PR (no sleep) | 50/50 clean | clean |

`test/js/node/worker_threads/worker-shutdown-post-leak.test.ts` runs the
worker-stdio-on-exit scenario under `detect_leaks=1` as an ASAN-lane
guard (in a fresh file so it actually runs; `worker_destruction.test.ts`
is ASAN-quarantined via `test/expectations.txt`). The race is not
observable on the debug gate without `src/` instrumentation, so the
fail-before half will not fire there; `test-worker-stdio-flush.js` on
the release-asan lane remains the primary signal.

Related: oven-sh#31216 (introduced the ordering), oven-sh#29917 (described but left
the remaining window).

<!-- 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/worker_threads/worker-shutdown-post-leak.test.ts

<!-- robobun:evidence:end -->
springmin pushed a commit that referenced this pull request Jul 23, 2026
…and empty <failure> (oven-sh#34975)

### What does this PR do?

Fixes three problems with the JUnit XML reporter that, together, made
the report unparseable and stripped of any useful failure information.

#### Repro

```sh
cat > x.test.js <<'JS'
describe('suite <a> & "b"', () => {
  test("plain fail", () => { throw new Error("boom: important message"); });
  test("ctrl \x00nul\x1besc", () => { throw new Error("x"); });
});
JS
bun test --reporter=junit --reporter-outfile=r.xml
python3 -c "import xml.etree.ElementTree as ET; ET.parse('r.xml')"
# xml.etree.ElementTree.ParseError: not well-formed (invalid token): line 8, column 36
```

#### 1. Control characters in test names produce malformed XML

`escape_xml` wrote `&#N;` for every C0 control byte but neither flushed
the pending run nor advanced `last`, so the raw byte was *also* emitted,
and the references appeared at the start of the string instead of in
place. Since bytes 0x00..=0x08 / 0x0B / 0x0C / 0x0E..=0x1F are not legal
XML 1.0 `Char`s even as numeric references, the fix passes TAB/LF/CR
through and drops every other C0 byte.

Before: `name="&#0;&oven-sh#27;ctrl ^@nul^[esc"` (parse error).
After: `name="ctrl nulesc"` (parses).

#### 2. `classname` is double-escaped

The describe-scope names were XML-escaped while being joined with `"
&gt; "`, then escaped again inside `write_test_case`, so
`describe('suite <a> & "b"')` produced `classname="suite
&amp;lt;a&amp;gt; &amp;amp; &amp;quot;b&amp;quot;"`. The inner
`<testsuite name="">` was only escaped once, so the two disagreed. The
join now concatenates the raw names with a raw `" > "` separator and
leaves the single escape to `write_test_case`.

#### 3. `<failure>` carries no message, stack, or correct type

Every thrown error was reported as `<failure type="AssertionError" />`
with no message or stack. `on_uncaught_exception` now records the
error's name, message, and a colourless `print_errorlike_object`
rendering on the `JunitReporter`; `write_test_case` emits them as the
`type` and `message` attributes and the element body:

```xml
<failure type="Error" message="boom: important message">1 | describe(...
   ...
error: boom: important message
      at &lt;anonymous&gt; (x.test.js:3:71)
</failure>
```

`TypeError`, `RangeError`, etc. now report their real name in `type`.
Timeouts also gain `message="test timed out"`.

### How did you verify your code works?

New tests in `test/js/junit-reporter/junit.test.js`:

- `produces well-formed XML when test names contain control characters`:
asserts the raw bytes contain no illegal C0 characters, no
`&#0;`/`&oven-sh#27;`, the report parses with a strict XML parser, and TAB/LF
survive.
- `escapes the classname attribute exactly once`: asserts no `&amp;lt;`
/ `&amp;amp;` etc. appear and the decoded `classname` round-trips to the
original describe titles.
- `includes the error type, message and stack in <failure>`: asserts
`type`/`message` and body text for a plain `Error`, a `TypeError`, and
an `expect().toBe()` failure, and that no ANSI escapes leak into the
report.

All three fail against `main` and pass with this change. The existing
`junit reporter` tests and the `--parallel --reporter=junit` tests still
pass.

<!-- robobun:evidence:begin -->

---

**[review]** gate passed · iteration 5 · 6 files touched

<details><summary>fails on main (without fix)</summary>

```console
ASAN without fix: BUILD FAILED (no junit output)
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/junit-reporter/junit.test.js
ninja: Entering directory `/workspace/bun/build/debug'
[1/184] gen bindgenv2
[2/184] gen bake.{client,server,error}.js
-> bake.client.js, bake.server.js, bake.error.js
[3/184] gen generated_host_exports.rs
generated_host_exports.rs: 91 exports (host=3, lazy=10, generic=78, rust=0); 240 extern-C blocks audited
[4/184] gen cpp.rs (cppbind)
[5/184] gen ZigGeneratedClasses.{cpp,h,rs}
Found 2 classes from /workspace/bun/src/jsc/resolve_message.classes.ts
  - ResolveMessage (13 fields)
  - BuildMessage (10 fields)
Found 1 classes from /workspace/bun/src/runtime/api/Archive.classes.ts
  - Archive (4 fields, 1 class fields)
Found 2 classes from /workspace/bun/src/runtime/api/BunObject.classes.ts
  - ResourceUsage (8 fields)
  - Subprocess (20 fields)
Found 1 classes from /workspace/bun/src/runtime/api/cron.classes.ts
  - CronJob (5 fields)
Found 3 classes from /workspace/bun/src/runtime/api/filesystem_router.classes.ts
  - FileSystemRouter (5 fields)
  - FrameworkFileSystemRouter (2 fields)
  - MatchedRoute
... (truncated)

release without fix: 6 FAILED
bun test v1.4.0-canary.1 (1498d7b)

test/js/junit-reporter/junit.test.js:
(pass) junit reporter > should generate valid junit xml for passing tests %s [20.52ms]
(pass) junit reporter > should generate valid junit xml for passing tests %s [15.13ms]
/tmp/junit-comprehensive_5eGMij
253 |       stderr: "pipe",
254 |     });
255 |     await proc1.exited;
256 | 
257 |     const xmlContent1 = await file(junitPath1).text();
258 |     expect(filterJunitXmlOutput(xmlContent1)).toMatchSnapshot();
                                                    ^
error: expect(received).toMatchSnapshot(expected)

@@ -2,26 +2,26 @@
  "
  
    
      
        
-         
+         
        
        
-         
+         
        
        
-         
+         
        
        
-         
+         
            
          
        
        
        
-         AssertionError: expect(received).toBe(expected)&#10;&#10;Expected: 3&#10;Received: 2&#10;&#10;      at comprehensive.test.js:31:27&#10;
+         
        
        
          
        
        

- Expected  - 5
+ Received  + 5

      at <anonymous> (/workspace/bun/test/js/junit-reporter/junit.test.js:258:47)
(fail) junit reporter > mor
... (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/mechgate.xml" test/js/junit-reporter/junit.test.js
bun test v1.4.0 (a93ab8b)

test/js/junit-reporter/junit.test.js:
(pass) junit reporter > should generate valid junit xml for passing tests %s [796.77ms]
(pass) junit reporter > should generate valid junit xml for passing tests %s [576.89ms]
/tmp/junit-comprehensive_Py8zho
(pass) junit reporter > more scenarios [1138.97ms]
(pass) junit reporter > should report only the final result for a retried test [652.04ms]
(pass) junit reporter > produces well-formed XML when test names contain control characters [531.82ms]
(pass) junit reporter > escapes the classname attribute exactly once [467.97ms]
(pass) junit reporter > keeps the test body's error in <failure> when afterEach also throws [456.76ms]
(pass) junit reporter > includes the error type, message and stack in <failure> [525.69ms]

 8 pass
 0 fail
 2 snapshots, 135 expect() calls
Ran 8 tests across 1 file. [8.06s]
__F:0:S:0

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 882ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/142] gen bindgenv2
[2/142] gen bake.{client,server,error}.js
-> bake.client.js, bake.server.js, bake.error.js
[3/142] gen generated_host_exports.rs
generated_host_exports.rs: 91 exports (host=3, lazy=10, generic=78, rust=0); 244 extern-C blocks audited
[4/142] gen cpp.rs (cppbind)
[5/142] gen ZigGeneratedClasses.{cpp,h,rs}
Found 2 classes from /workspace/bun/src/jsc/resolve_message.classes.ts
  - ResolveMessage (13 fields)
  - BuildMessage (10 fields)
Found 1 classes from /workspace/bun/src/runtime/api/Archive.classes.ts
  - Archive (4 fields, 1 class fields)
Found 2 classes from /workspace/bun/src/runtime/api/BunObject.classes.ts
  - ResourceUsage (8 fields)
  - Subprocess (20 fields)
Found 1 classes from /workspace/bun/src/runtime/api/cron.classes.ts
  - CronJob (5 fields)
Found 3 classes from /workspace/bun/src/runtime/api/filesystem_router.classes.ts
  - FileSystemRouter (5 fields)
  - FrameworkFileSystemRouter (2 fields)
  - MatchedRoute (8 fields)
Found 1 classes from /workspace/bun/src/runtime/ap
... (truncated)
```

</details>

<details><summary>diff hotspot</summary>

```
src/jsc/VirtualMachine.rs                          |  10 ++
 src/runtime/cli/test_command.rs                    | 181 +++++++++++++++++++--
 src/runtime/test_runner/Execution.rs               |  18 ++
 src/runtime/test_runner/bun_test.rs                |  27 ++-
 .../__snapshots__/junit.test.js.snap               |  18 +-
 test/js/junit-reporter/junit.test.js               | 172 +++++++++++++++++++-
 6 files changed, 401 insertions(+), 25 deletions(-)
```

</details>

**gate history** · 6 passed · 1 rejected · iteration 5

<details><summary>evidence per changed file</summary>

```
file                                                     reads  edits  tests
src/jsc/VirtualMachine.rs                                    8      2      0
src/runtime/cli/test_command.rs                             12     14      0
src/runtime/test_runner/Execution.rs                         4      3      0
src/runtime/test_runner/bun_test.rs                          6      8      0
test/js/junit-reporter/__snapshots__/junit.test.js.snap      0      0      0
test/js/junit-reporter/junit.test.js                         7     16      0
```

</details>

<!-- robobun:evidence:end -->
springmin pushed a commit that referenced this pull request Jul 29, 2026
…rier (oven-sh#36337)

`JSNativeStreamSourceAdapter::m_controller` was a
`JSC::Weak<JSReadableStreamDefaultController>`. When the native pull
promise is rejected (socket fault on a fetch body) the adapter is queued
as the `onNativePullRejected` reaction context, which roots the
**adapter** but not the **controller**: the adapter's only edge to it
was the `Weak`. `FetchTasklet` releases both native `Strong<>`s to the
body stream before that microtask drains, so a GC in between can leave
the entire consumer graph (`controller -> stream -> reader -> pipe op ->
destination -> writer -> readyPromise`) white. The subsequent error
cascade then enqueues the pipe's writes-drained shutdown deferral
against a corpse `op`, and `performPipeShutdownAction(AbortDestination)`
dereferences a swept `readyPromise`:

```
ASSERTION FAILED: result   JSObject.h(583) JSGlobalObject *JSC::JSObject::realm() const
#5  JSC::JSObject::realm()
#6  JSC::JSPromise::rejectPromise
#7  JSC::JSPromise::reject
#8  Bun::WebStreams::writableStreamDefaultWriterEnsureReadyPromiseRejected
#9  Bun::WebStreams::writableStreamStartErroring
#10 Bun::WebStreams::writableStreamAbort
#11 WebCore::performPipeShutdownAction (AbortDestination)
#12 WebCore::JSStreamPipeToOperation::onWritesFinishedForShutdown
```

On builds without the assert the same path is a silent write into
freed/reused promise memory.

## Fix

Hold `m_controller` as a visited internal field so a queued adapter
roots the controller directly. The edge is cleared on every terminal
path (`nativeSourcePullRejected`, `nativeSourceCallClose`,
`nativeSourceCancel`); `controller->algorithmContext` is cleared by
`readableStreamDefaultControllerClearAlgorithms`, so the abandoned case
is an ordinary intra-heap cycle mark-sweep collects.
`NewSource::this_jsvalue` is only `Strong` during FileReader I/O, where
pinning the consumer graph is the correct behavior anyway.

With the `Weak` gone the adapter no longer needs a destructor, so it is
now a `JSInternalFieldObjectImpl<5>`: the five JSValue members (handle,
pendingView, closer, drainValue, controller) are internal fields visited
by the base class, with typed accessors at call sites. The scalar
members (chunkSize, flag bitfield, text-decode state) stay as plain
members.

## Verification

`native-source-onclose-leak.test.ts` (the partial-read + `releaseLock`
abandonment tests for Blob/fetch/File sources) continues to pass,
confirming the cycle does not pin. `streams.test.js`,
`pipeTo-signal-leak.test.ts`, `compression.test.ts`, `blob.test.ts` all
pass.

The crash itself is 0/1800 standalone; it reproduces ~1/3 only under a
fault-injected tracer replay. `pipeTo-shutdown-gc.test.ts` exercises the
shape (native body source, socket fault mid-stream, fire-and-forget
`pipeTo` under `collectContinuously`, `AbortDestination` shutdown arm)
as a regression surface.

<!-- 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/web/streams/pipeTo-shutdown-gc.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 -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants