test(node): clearImmediate semantics across TCP socket close - #34852
Merged
Conversation
Add a spec test that mirrors the `npm:postgres` (postgres-js) write
batching pattern that's been surfacing in user crashes:
function write(x) {
chunk = concat(chunk, x)
if (nextWriteTimer === null)
nextWriteTimer = setImmediate(nextWrite)
}
function nextWrite() {
socket.write(chunk)
clearImmediate(nextWriteTimer)
chunk = nextWriteTimer = null
}
function closed() {
clearImmediate(nextWriteTimer)
socket = null
}
The test pins down four invariants that Deno already implements but that
the issue reporter expected to behave differently:
1. `clearImmediate(t)` called synchronously cancels `t`.
2. `clearImmediate(t)` called from a microtask before any check phase
runs cancels `t`.
3. When a `setImmediate` is queued in the same tick that the socket is
torn down, libuv's check phase (immediates) runs BEFORE the close
phase. The `'close'` handler's `clearImmediate` therefore observes
that the immediate has already fired — identical to Node.js.
4. The crash that #34667 hits ("Cannot read properties of null (reading
'write')") originates from postgres-js's connection pool re-running
`write()` AFTER `closed()` has nulled `socket`. A `socket && ...`
guard inside `nextWrite` — the fix in porsager/postgres#1168 — keeps
the queued callback from dereferencing the nulled socket. The same
guarded pattern is verified here to ensure Deno never regresses
against it.
Refs #34667.
Co-Authored-By: Divy Srivastava <me@littledivy.com>
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.
Summary
Adds a spec regression test that pins down
setImmediate/clearImmediatesemantics around a TCP socket's
'close'event. The test directly mirrors thewrite-batching pattern that surfaces in the crash reported in #34667 (and the
duplicate trail of #29262 sightings against
npm:postgres):Investigation
I reproduced the exact stack trace from #34667 with a minimal pure-
node:netscript (no
npm:postgresneeded) and confirmed it crashes identically onboth Deno and Node.js 24.15.0 — same stack, same
TypeError: Cannot read properties of null (reading 'write')from insideImmediate.callback. Sothe symptom is not Deno-specific; it's a userland race in postgres-js's
connection-pool reuse path that surfaces on every libuv-compatible runtime.
The root cause (already triaged upstream as porsager/postgres#1066 and #1154):
write()queuessetImmediate(nextWrite). That immediateruns once, in the check phase of the same tick — long before any
server-side close arrives.
nextWriteTimeris reset tonullinside theimmediate.
callback emits
'close'. postgres-js'sclosed()callsclearImmediate(nextWriteTimer)— but the timer is alreadynull, so it'sa no-op — then sets
socket = null.await slowcontinuation runs in themicrotask drain that follows the close handler, then the user calls
conn.release()→ which moves the now-dead connection into the pool'sopenqueue →sql.reserve()shifts it back out → the nextINSERTtriggers
c.execute(q)→write(insertBytes)→ a freshsetImmediate(nextWrite)is queued.socket === null,so
socket.write(chunk, fn)throws — and because the throw is inside acheck-phase callback, no surrounding
try/catchin user code can catchit. The process exits.
Step 3 is the actual bug. The upstream fix is porsager/postgres#1168, which
adds a
socket && socket.write(chunk, fn)guard insidenextWrite. It's beenopen since 2026-05-25 awaiting maintainer review, with at least one user already
applying it as a
pnpm patchto keep their service stable.What this PR adds
A spec test under
tests/specs/node/clear_immediate_socket_close_race/thatpins down four invariants Deno already satisfies, so we never silently regress
against the actual contract reporters were expecting:
clearImmediate(t)called synchronously cancelst.clearImmediate(t)called from a microtask before any check phase runscancels
t.setImmediateis queued in the same tick that the socket is torndown, libuv's check phase runs before the close phase — so the queued
immediate fires first and the close handler's
clearImmediateis a no-op,identical to Node.js. (The reporter assumed the opposite order, which is
the heart of the misdiagnosis.)
socket && …guard insidenextWrite(matching upstream Rewrite libdeno/snapshot_creator.cc in Rust #1168),the pool-reuse-after-close pattern stops crashing on Deno.
What this PR does NOT fix
The actual crash for users still on
postgres@3.4.9(or anything pre-#1168).That fix has to land in
porsager/postgresand propagate via an npm release —Deno can't patch user libraries in-tree. Pointing the issue at upstream so users
can apply the
pnpm patchworkaround in the meantime.Test plan
tests/specs/node/clear_immediate_socket_close_race/passes locally.socket && …guard from case 4 reproduces the exactUncaught TypeError: Cannot read properties of null (reading 'write')stack from the issue.
confirming this is not a Deno-specific bug.
deno fmt+deno lintclean on the new file.Refs #34667
Refs porsager/postgres#1168
Closes denoland/divybot#465