Repository navigation
Conversation
The server subprocess writes STOPPED to stderr and sends CONNECTION_CLOSE in the same turn. The two reach the test on different threads. When the rejection of the fetch arrives first, the promise has no handler yet and the runner fails the test with the HTTP3StreamReset error that the test expects. Attach the handlers when the fetch is created. The wait for STOPPED and the assertion do not change.
|
Status: ready for review. The diff is green. Each CI build is red only because of one test in a file that this PR does not change. How I reproduced the failure:
CI ran two times on the same diff. In each build 180 of 181 jobs passed, and no lane reports
The lane that is red in one build is green in the other, so every lane passed with this diff at least one time. I do not plan another CI run. The 5 s timeouts of this file under load are a separate problem. This PR does not change them. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: oven-sh/bun/.coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review. WalkthroughThe HTTP/3 server stop test now records the ChangesHTTP/3 stop test
Suggested reviewers: Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The change addresses the reported unhandled rejection in the HTTP/3 stop test without changing production behavior. The separate load-related timeouts are unchanged; no merge-blocking risk is identified. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Updated 12:42 PM PT - Sep 27th, 2026
❌ @robobun, your commit 716ba24 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 44146That installs a local version of the PR into your bun-44146 --bun |
…ear and cannot corrupt them (oven-sh#41973) ### Problem - `S.A + "k" + S.A + "k" + ...` over an inlined string enum member folds in quadratic memory: 8 192 terms (49 KB) take 1.4 GB in `bun run`. Every `A.B` reference shares the member's rope, so `join_strings` (`src/ast/fold_string_addition.rs`) deep-cloned both ropes when either operand was an `E::InlinedEnum`: each enum term re-cloned the accumulator. - `Template::fold` (`src/ast/e.rs`) and members named with `*/` (left unwrapped by `wrap_inlined_enum`) had no guard and appended onto the shared rope. ``enum A { B = "1" + "2", T = `t${B}t` }`` makes `A.B` print `"12t"`. A second use panics on 1.4.3: `called Option::unwrap() on a None value`, `Crashed while visiting`. ### Fix - `s_enum` flattens the member's string (`resolve_rope_if_needed`) before it stores the `EnumString`, the only place one is built. A shared string is then never a rope. - `join_strings` loses `has_inlined_enum_poison` and `clone_rope_nodes` and links in O(1). 4 096 pairs: 1 545 MB → 9 MB. - The src commits are oven-sh#38998 unchanged. This supersedes it, adds the memory test, and bumps the transpiler cache version since affected output changes. - Verified: `test/js/bun/transpiler/transpiler-enum-concat-chain-oom.test.ts` (new, fails on 1.4.3), the oven-sh#38998 tests, the enum suites. Self-reviewed: 2 concerns raised, 2 addressed. ### Background - String folding copies no bytes. `"a" + "b"` links the right `EString` node onto the left through `next`/`end` (a rope). `EString::push` writes to the rope's last node, so a rope needs one owner. - The visit turns `A.B` into an `E::InlinedEnum` around a copy of the member's root node. Later folds see a string literal. <details><summary>Notes</summary> - Fuzz-ledger finding oven-sh#44146. Curve before: 1 024 pairs 62 MB, 4 096 700 MB, 8 192 2.76 GB, 16 384 OOM-killed at 3 GB (x2 terms, x4 memory). `bun build --minify-syntax` behaves the same. Not a regression: 1.3.14 behaves the same, and the Zig code this was ported from had the same shape. - The `*/` case on 1.4.3: `enum A { "*/" = "s" + "t" }; console.log(A["*/"] + "u", A["*/"] + "v")` panics the same way. With members stored flat the unwrapped value is a single node, so it is safe too. - After the fix, debug+ASAN build, both chains of the test (top level and inside an enum body): 1 024 pairs 7 MB, 4 096 9 MB, 16 384 22 MB, 65 536 70 MB. `bun run` of a 16 384-pair file peaks 8 MB over an empty script. - I first narrowed the clone to the side that is the enum member (also linear). Storing members flat is simpler: it removes the clone, covers `Template::fold`, the `*/` names and the bundler's cross-module substitution at the one place the sharing starts, and costs one O(len) copy per rope-valued member. esbuild stores enum string values flat too. - `resolve_rope_if_needed` leaves `rope_len` (still the length) and a stale `end` behind. `end` is only read from a node whose `next` is set, and a push onto a copy of a flat node assigns both. - The runtime transpiler cache is keyed on the source and the features hash, not on bun's version. A file that hit the template corruption without the panic was cached with the wrong output, so `EXPECTED_VERSION` goes to 30. - Out of scope, tracked separately: a `+` chain of template literals that each have a substitution (`` `a${x}b` + `a${x}b` + ... ``) is also quadratic under syntax minification (4 096 terms: 526 MB). That is `concat_parts` re-copying the accumulated `parts` array at each step, a different path that this change does not touch. - Suites run on the debug build: `bundler_edgecase.test.ts`, `bundler_minify.test.ts`, `bundler_string.test.ts`, `esbuild/ts.test.ts`, `esbuild/default.test.ts` (enum/template/string filter), `transpiler/transpiler.test.js`, `transpiler-comma-chain-oom.test.ts`, `cli/run/transpiler-cache.test.ts`. `cargo clippy` on `bun_ast` and `bun_js_parser` is clean. </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/bun/transpiler/transpiler-enum-concat-chain-oom.test.ts <!-- robobun:evidence:end -->
|
@robobun wake up!! |
|
@robobun wake up!! |
|
I am here. This PR is ready, and it waits for a maintainer.
I did not start CI again. I already pushed one empty commit to re-run it, and I do not push a second one. What the PR needs from you:
|
Problem
server.stop(true) %s inside an H3 handler sends CONNECTION_CLOSEintest/js/bun/http/serve-http3.test.tscan fail on the error that it expects. The runner printsTypeError: HTTP3StreamReset fetching "https://127.0.0.1:<port>/stop"and then(fail).STOPPEDline of the server (serve-http3.test.ts:1019-1029). The server writesSTOPPEDand sends CONNECTION_CLOSE in the same turn. When the rejection arrives first, the promise has no handler.Fix
.then(r => r.text()).catch(e => e.code)when the fetch is created. Nothing else changes.Background
STOPPEDline arrive by independent paths.bun testfails the running test when a rejected promise has no handler after the microtask queue drains. A handler attached in a later task is too late.Notes
The 5 s timeouts. This PR does not change them. Under the debug build they hit 5 of 30 executions of the old test and 5 of 30 of the new test, in the same minutes. In the instrumented runs the time went to the first request on a new connection, before the changed lines run. #41564 and #41497 cover the
HTTP3HandshakeFailedfailure under load.Mechanism probe. A copy of the test whose server holds the
STOPPEDline back (setTimeout(() => console.error("STOPPED"), 300)afterserver.stop(true)):TypeError: HTTP3StreamResetoutput aboveNatural order, no delay. A copy of the test with a timestamp per phase shows the order in each unhandled rejection failure. The
(fail)line comes first and theSTOPPEDline arrives after it (for example fail at 186 ms,STOPPEDseen at 276 ms).Interleaved runs. The file from
mainagainst the file from this branch, filter-t 'server.stop\(true\) (synchronously|after an await) inside an H3 handler', 2 executions per run. Host: Linux x64, 16 cores, load average between 300 and 900 during the runs.1.4.3-canary.1+367d939d9, 50 runs1.4.3-canary.1+367d939d9, 50 runsWhole file. One run under the debug build on the same loaded host: 60 pass, 13 fail. The two changed cases pass. All 13 failures are 5 s timeouts in the first
describeblock. The rejections that arrive after each timeout carryHTTP3HandshakeFailed. I did not run the file on a quiet host.Other promises in the file that are awaited late.
client RST mid-/big does not break the listener:.catchis attached in the same turn.server.stop() inside an H3 handler drains the in-flight requestandgraceful stop: in-flight H3 requests complete after server.stop(): they expect fulfilment, so a rejection is a real failure.req.signal aborts on client RST: the promise rejects only after the test callsac.abort(), and the handler is attached in that turn.The chain.
.catchstays after.then, as before, so it also handles a rejection ofr.text(). The promise that the test holds cannot reject.The runner check, with the release build. A handler attached in the same turn or after
await nullpasses. A handler attached aftersetImmediatefails the test with the rejection.CI. The file is listed in
test/flaky-tests.txt. CI retries a file that fails and annotates a pass on the retry as flaky.[auto-merge] gate passed · iteration 1 · 1 files touched
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 1 rejected · iteration 1
evidence per changed file