Repository navigation
Conversation
… task A send() the kernel rejects latches transport_write_fatal, and the deferred auto-flush tick closed the socket. The deferred task queue runs whenever the outermost JS call returns, which can be a dispatch made from on_native_read. The close then detached the socket under its own read, the rest of the read wrote through the detached transport, and the session reported that write's error: EBADF on a client, ERR_SOCKET_CLOSED on a server stream, which ends the process when the stream has no 'error' listener. The close also runs JS close handlers that finish from process.nextTick, and JS that runs inside the queue gets no checkpoint of its own. With the socket as the last handle the process exited 0 with no 'close' on the stream or the session. The deferred tick now queues an event loop task that does the close, the way FileSink::run_pending_later leaves the deferred queue. The task holds a parser ref, which ManagedTask drops if the loop releases the task unrun, and it decides again when it runs. On Windows a queued task does not shorten the libuv poll, so the loop is woken.
Collaborator
Author
|
Updated 10:31 AM PT - Oct 15th, 2026
❌ @robobun, your commit 5eb65e5 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 42485That installs a local version of the PR into your bun-42485 --bun |
Collaborator
Author
StatusReproduced on main @6a92015f (linux-x64 debug), on the 1.4.3-canary release binary, and on the Windows x64 canary. One process holds both ends of a plaintext HTTP/2 connection. The peer resets, and the client calls A server that closes its listener with a request in flight, and whose response write then fails inside the stream's
|
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 8 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe parser queues deduplicated fatal-write transport closures as event-loop tasks. The task rechecks fatal-write and backpressure state before closing. Regression tests cover HTTP/2 client and server cleanup after peer resets and injected ChangesHTTP/2 fatal write cleanup
Suggested reviewers: Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check |  |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Title check | |
The title clearly and concisely describes the main change: moving fatal-send transport closure to an event-loop task for node:http2. |
| Description check | |
The description is detailed and covers the problem, fix, scope, verification, test results, and implementation notes. It does not use the template headings exactly, but it provides the required inform… |
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
The server case reacts to the child's stdout line by line, and it started to read stderr only after stdout closed. A child that fills the stderr pipe would block and the test would hang. Read both pipes at once, and assert the report, stderr and the exit code as one object in all four cases.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Collaborator
Author
|
A data point for this change from #42182 (close client sessions with the send errno). One case there depends on where this close runs.
|
…2-fatal-send-close-task
#43675 removed ManagedTask. FatalWriteCloseTask now implements Taskable under the new H2FatalWriteClose tag. release_unrun drops the box, so a VM that stops before the task runs still gives the parser ref back.
Collaborator
Author
|
Merged main (
Not run again after the port: the |
…2-fatal-send-close-task
Jarred-Sumner
added a commit
that referenced
this pull request
Oct 21, 2026
…6270) ### What does this PR do? Hardening follow-up to #46021. It fixes regressions from #46021 and #44618 that were never released, includes more of the open pull requests that fix a crash, and fixes a few crashes that had no pull request. #### Regressions fixed (none was released) - `spyOn(Bun, "$")`, and the same for `"sql"`, `"postgres"` and `"SQL"`, threw "does not support accessor properties yet". - Async `fs.read`, `fs.write` and `zlib.brotliCompress` on a view over a `WebAssembly.Memory` could lose data or copy far more than they needed. - `node:vm` with `vm.constants.DONT_CONTEXTIFY`: `this` in host accessors, prototype handling, `util.types.isProxy`, `Bun.deepEquals`, `assert.deepEqual` and `console.log` of a context. - `zlib` `stream.flush()` threw for arguments it used to accept, and streams had a new enumerable property. - `Bun.Transpiler` `exports.replace` with the name `"default"`. - `process.env.X = 3000` could store the number once the function was JIT-compiled. - `node:sqlite` and `bun:sqlite` could lose rows at exit. - Dev server: stylesheets shared between pages after a build error. - Shell `cp` of a file onto itself exited 1. - `error.stack`: the frame of `require()` reads `at anonymous (unknown)` again. - `import * as process from "process"` followed by `process.on(...)` threw at load. The EventEmitter methods that the module exports now act on `process`. - `delete res.statusCode; res.end()` in a `node:http` handler threw. It sends 200, as in Node. (from #46128) - `node:tls`, `node:https` and `node:http2`: connection teardown and write callbacks behave as in 1.4.3 and Node again. #### Crash fixes Bundler, parser and CSS: #42286, #46092, #46097, #46177, #46239, #36334, #43024, #40769. `bun test`, `--hot` and `--watch`: #30776, #38206, #38974. Shell: #43183, #37743, #46168, #46207, #34900. `node:http`, `node:http2`, `Bun.serve`, streams: #43449, #45380, #44457, #42197, #42485, #38082, #38124, #46074, #45999, #46045. `node:vm`, native addons, workers: #46129, #46198, #45615, #33412, #38031. Dev server: #42790, #42850, #39488. Other: #35991, #38030, #38586, #40526, #41830, #46029, #46078, #46114, and the bug of #42215. #### Behavior changes - `crypto.randomFill(view, cb)` reports `ERR_CRYPTO_OPERATION_FAILED` if it could not fill the view. It used to report success. - A function registered twice on `process` runs twice, as in Node. (#41830) - Under `--hot`, a reload after an uncaught exception cancels the timers that the failed run created. The new run's timers fire, and so do timers and `fs.watchFile` watchers that earlier runs kept on `globalThis`. The reload also resets the exit code to 0. (#38206) - The parent of a Worker no longer gets an `'error'` event for a failed `Bun.serve` request in that Worker. The error is printed, as on the main thread. - `hash.write()` after `digest()` drops the chunk, as in Node. (#35991) - `http2` over TLS with `server.emit("connection", socket)`: `session.socket` is a `TLSSocket` and a `'connection'` listener fires, as in Node. (#38124) - A `slice()` with no type of a body `Blob` has `type === ""`, as in Node. (#46114) - `util.types.isProxy(globalThis)` is false, as in Node. - A write on a `node:net` or `node:tls` socket that is still pending when the connection closes calls back with `ECANCELED`, as in Node. - `HTMLRewriter.on()` throws for a selector that is nested too deeply. - `Bun.Image`: `maxPixels` also bounds PNG metadata, `metadata()` of a GIF reports the size that gets decoded, and a path with a NUL byte throws. (#40526) - `bun build --no-bundle` with the `base64` or `dataurl` loader prints an error. (#36334) - Bundled output for a default-exported function or class expression changes to `var file_default = function name() {}`, as in esbuild. `.name` is unchanged. (#46097) - A Proxy in the prototype chain of a `node:vm` global is allowed. jsdom 30 with `runScripts` needs this. - `bun:sqlite` commits a write that `iterate()` left open when the main thread exits. An explicit `BEGIN` is still rolled back. - When a Worker stops while a native addon's async work is still running, the addon's finalizers run after that work is done, as in Node. - Shell: `mv -n` and `cp -n` don't overwrite, and `cp` reads `R`, `v` and `n` anywhere in a flag group. #### Performance - Ordinary `node:vm` contexts run at the same speed. With `DONT_CONTEXTIFY`, stores to globals and reads of inherited names in a hot loop cost 10 to 20 % more than on `main`. - A `process.env` write costs 20 to 35 % less than after #46021. - An async read into a `WebAssembly.Memory` view looks through the live memories: about 1.5 µs more with one memory. #### Not included - Taken in part, so these should stay open for the rest: #36334, #38974, #39488, #40526, #40769, #42485, #45615, #45999, #46029, #46092, #46114, #46168, #46207, #46239. - Needs a WebKit change: #34655 #39204 #40170 #40253 #41587 #41900 #42699 #42897 #42903 #43084 #43813 #44518 #44949 #45770 #45804 #45827. - #44723 (covered by #46097), #41675, #42753, #38762, #41475, #46016. ### How did you verify your code works? - Every item was reproduced first on release builds of `main`, of its parent and of 1.4.3, and compared with Node v26 where Node has the API. - Every fix has a test that fails on `main` and passes on a debug + ASAN build of this branch, except where that isn't practical (huge inputs, code that only runs on Windows or macOS). - The regressions were checked on grids of calls against the build before #46021 and against 1.4.3. - Bun's own test suites and Node's `test-vm-*`, `test-http2-*` and `test-zlib-*` files for the areas touched.
This branch has not been deployed
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.
Problem
send()after a peer reset, a plaintextnode:http2session closes its transport inside the deferred task queue (src/runtime/api/bun/h2_frame_parser.rs:3072). Regression in 1.4.0 (node:http/https/http2: raise Node v26.3.0 compat to ~94%, sync the upstream suites, and fix the Windows/macOS transport-layer teardown bugs they exposed #32488).on_native_read. The rest of the read then writes through the detached transport: a client reportsEBADF, and a server stream with no'error'listener ends the process withERR_SOCKET_CLOSED.process.nextTick, and no checkpoint follows the deferred tasks (src/jsc/event_loop.rs:424-437). With the socket as the last handle, the process exits 0 with no'close'event.Fix
FatalWriteCloseTask) that closes the socket, asFileSink::run_pending_laterdoes.test/js/node/http2/node-http2-syscall-fault.test.ts, four new cases: two with a real reset (fail on 1.4.3 and main), two with fault injection (fail on main'ssrc/). Alsotest/js/node/http2/, node's 261test-http2-*files, grpc-js.Background
H2FrameParseris the native HTTP/2 engine behind anHttp2Session.EventLoop::exit()skips its checkpoint while it runs.transport_write_fatalis the latch a rejectedsend()sets. The parser keeps TLS writes off it (tls: consolidate the open TLS fixes (both engines, node:tls, node:https, WebSocket, SQL) #44618), so only h2c sessions set it.Notes
Scope. Plaintext HTTP/2 only (
http2.connect("http://..."),http2.createServer(), gRPC over insecure channels). A TLS session never sets the latch. Until #44618 a TLS write reported no fatal error. Since #44618 it does, and the parser clamps that result to -1 inNativeSocketWrite for &TLSSocket. The comment there says the TLS layer closes the socket after it reads what the peer sent. No user reported this. It was found during the work on #42182.What main does, measured (main @6a92015f, linux-x64 debug, and 1.4.3-canary release. The two client cases below behave the same on the Windows x64 canary).
Client, peer resets at the preface (peer under node, client under bun, no timer in the client), 3/3:
Client, one process holds both ends, the peer resets and the client calls
request()in the same turn:Server, the listener is closed while a request is in flight, then the response write fails:
The clean close (
rstCode8, no'error') is what main already reports when something else wakes the loop, and it is what Node'ssocketOnClosedoes to open streams (NGHTTP2_CANCEL). #42182 changes the client result toECONNRESET. This PR does not change which result is reported, only that it is delivered.Why the read callback case breaks on main.
rewrite_readdispatches a frame event to JS. The handler writes, the send fails, and the latch registers the auto flusher. When the handler returns, the outermostexit()runs the checkpoint and then the deferred tasks, so the fatal branch closes the socket whileon_native_readfor that same socket is still on the stack. The close detaches the native socket.rewrite_readthen ends withself.flush(), which finds no native socket and goes through the JSwritehandler.net.Socket._writefails that write, the session takes it as a transport error and destroys its streams with it.Relation to #42353, and the order. #42353 (
node:http2: submit a request's frames after request() returns) drains pending microtasks again afterdeferred_tasks.run(). With it, theprocess.nextTickfollow-up of a close made inside the queue runs, so the lost-events half of this bug goes away for every deferred task. Itson_auto_flushstill callsclose_transport_after_fatal_write()directly, so the close still runs underon_native_read, and theEBADFandERR_SOCKET_CLOSEDresults stay. The two changes compose. #42353 deletes thehas_backpressure()block that this PR edits, so the second one to land has a small conflict there: keepqueue_transport_close_after_fatal_write()in the place of the direct close. If this PR lands first, all four new cases fail before it and pass after it. If #42353 lands first, the two cases that lose every event pass without this change, and the two read callback cases still need it.Other open PRs. #42182 (close client sessions with the send errno) edits
close_transport_after_fatal_writeand adds a line to the recovered-send branch. The task's own recovered-send branch then needs the same line. #37782 edits the condition of the fatal branch. #42197 addsBunSocket::Closed. None of these touches the line this PR changes.Ordering. The task runs in the next
tick(). When the deferred tick ran from an I/O callback or a timer, that is before the next poll. When it ran from asetImmediatecallback, one non-blocking poll comes first, so the read side can report the reset before the task runs. The task then finds the socket closed or detached and does nothing. On Linux the failedsend()has consumed the socket error, so that read is a clean EOF.State between the deferred tick and the task. The latch stays set, so later fatal results do nothing. Other paths can register the auto flusher again (
cork()), and the fatal branch then runs again:fatal_write_close_queuedmakes that a no-op. The task decides again when it runs, the way the deferred tick does: it closes if bytes are still buffered, and clears the latch if a later write drained them. A teardown releases the queued task through the task'sTaskable::release_unrun, which drops the boxed ref. Inrun, the queue's ref becomes a counted keepalive (OwnedKeepalive) before the close re-enters JS, sofinalizecan release it ifprocess.exit()strands the frame (release_refs_stranded_by_exit). Checked withBUN_DESTRUCT_VM_ON_EXIT=1and LeakSanitizer:process.exit()from inside the close dispatch reports no leak.The tests. All four cases are in one
describeinnode-http2-syscall-fault.test.ts. Every fixture reports atprocess.on("exit")and starts no timer. The asserts are Node-neutral:'close'on the stream and on the session, exit code 0, empty stderr, and no error other thanECONNRESET. The two real-reset cases need no fault injection, so they run on release lanes and on Windows (the file'safterEachnow callsfault.clear()only when the hooks exist). One process holds both ends, the peer resets and the client callsrequest()in the same turn, once fromsetImmediate(main: no event) and once inside'remoteSettings'(main:EBADF). Checked on Windows x64: the canary fails both, this branch passes both, 3/3 runs. The two injected cases usesocketFaultInjection(debug and ASAN builds, not Windows). The client case fails the second send of the connect flush, which is the reported scenario. The server case closes its listener with a request in flight and then writes the response from the stream's'data'handler (main: exit 1, uncaughtERR_SOCKET_CLOSED).Windows. A queued task makes the next poll non-blocking on POSIX only (
get_timeoutdropshas_pending_immediateon Windows, anduv_run(UV_RUN_ONCE)takes no timeout).auto_tickwakes libuv for immediates, not for tasks. Soqueue_transport_close_after_fatal_writecallswakeup()on Windows, for the case where it runs from asetImmediatecallback and the poll comes before the nexttick().Not changed. A send that fails inside the deferred tick itself (the usual corked path) latches and registers the auto flusher again, so the fatal branch runs at the next checkpoint, as before. From a
setImmediatecallback that checkpoint comes after the next poll. A dead socket has an event pending, so that poll returns at once. With an injected errno it waits for the next unrelated wake (measured: 0.3 to 0.8 s, bounded by the 4 s socket sweep).Self-review. Changes made after it: the doc comment of
queue_transport_close_after_fatal_writeleads with the stack reason, the task decides again when it runs, the loop is woken on Windows, the server test no longer waits for an unrelated wake,OwnedKeepalivereplaces anunsafeblock, and this text states the h2c scope and the relation to #42353. Left out: thepending_header_compression_errorbranch ofon_auto_flushalso dispatches JS from inside the queue. It is a separate path with its own latch, so this PR leaves it alone.Suites run on linux-x64 (debug):
test/js/node/http2/(all files),test/js/node/test/parallel/test-http2-*andsequential/test-http2-*(261 files pass),test/js/third_party/grpc-js(321 pass, the same 7 failures as main: 5 need DNS, 2 time out on debug builds),test/js/third_party/undici-h2.Port to main (10-08,
620b50f6ab). #43675 removedManagedTask.FatalWriteCloseTaskis now its own task type. It has theH2FatalWriteClosetag, aTaskableimpl, and one arm each inrun_taskand__bun_release_task_unrun(src/runtime/dispatch.rs). The task does the same work as before. On linux-x64 debug (ASAN),test/js/node/http2/gives 600 pass and 0 fail with a 60 s test timeout. A probe callsprocess.exit()while the task is queued, withBUN_DESTRUCT_VM_ON_EXIT=1and LeakSanitizer. It goes throughrelease_unrunand reports nothing. Not run again after the port: thetest-http2-*files, grpc-js, undici-h2, and Windows.Merge of main (10-15,
a407eae88a). A clean merge, with no change to this diff. On linux-x64 debug (ASAN): the four new cases fail with main'ssrc/and pass here.test/js/node/http2/gives 620 pass and 0 fail in 11 files, with the default timeout. Of node's 261test-http2-*files, 260 pass with theparallel/ones run four at a time. The other one,test-http2-forget-closed-streams.js(10,000 requests on one session), exceeded the 120 s limit of that run and passes alone in 101 s.bun checkandbun fmt --checkaccept the test file. Not run again: grpc-js, undici-h2, and Windows.no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/http2/node-http2-syscall-fault.test.ts