🧪 Keep the HTTP journal test's teardown off the fetch pool's clock - #411
Merged
Conversation
The HTTP-backend case binds the test server to an ephemeral port and sends Connection: close on every request. Deno's fetch client can open a spare keep-alive connection that never carries a request, and the node:http polyfill's close() waits for it — closeIdleConnections() only reaps sockets that finished a response, and close() destroys the reaper interval that would have expired it — so stop() waited on the client pool's 90s eviction interval (observed as an exactly-3m0s test) or, when eviction never came, past the verification deadline. A fixed default port also let concurrent suites share one listen port on macOS instead of refusing the second bind. Closes #407
PR #411: 🧪 Keep the HTTP journal test's teardown off the fetch pool's clock1 files, +20 / -1 Scope✅ PR scope looks good. Structural✅ No structural bloat detected. Slop✅ Slop indicators look low. Static Analysis✅ Oxlint found no issues. CorrectnessNo extraneous code patterns detected. |
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.
Why
packages/core/tests/guarded-journal.test.ts's HTTP-backend case intermittently stalled Deno test teardown — an exactly-3m0s test solo, and past the 20-minute deadline underdeno task verify's concurrent battery (#407). The stall is inDurableStreamTestServer.stop(): under Deno'snode:httppolyfill,server.close()waits for a connection only the same process's fetch pool can release.What changes
Before:
The test server bound the package's fixed default port 4437, and the HTTP client used the runtime's pooled keep-alive fetch. During the test's request burst, Deno's fetch client sometimes opens a spare connection that never carries a request. At teardown, the polyfill's
closeIdleConnections()refuses to reap it (it only destroys sockets that finished a response),close()tears down the 30s reaper interval that would have expired it, andstop()then waits on the client pool's eviction clock: ~180s when hyper-util's 90s idle-eviction interval fires (the "exactly 3m0s" runs), unbounded when it does not (the ≥20m battery timeouts; one wedgeddeno testprocess on the reproduction machine was 3.5 days old). The fixed port also let concurrent suites share one listen port on macOS — three runtimes' servers bound 127.0.0.1:4437 simultaneously withoutEADDRINUSEacross 120 overlapping runs.After:
The server binds an ephemeral port (
port: 0), and every request the test's client sends carriesConnection: close, so no connection outlives its response andstop()has nothing to wait on. Teardown still fully awaitsserver.stop(); the 20-minute deadline and the concurrent battery are untouched.How to verify it
deno test --allow-all --frozen packages/core/tests/guarded-journal.test.tsstalled at teardown and completed at exactly 3m0s — 3 of 3 iterations under concurrent 3-runtime load, 2 of 5 solo shortly after, 2 of 12 instrumented runs. A stalled process held both ends of one ESTABLISHED self-connection to 4437 with no listener, and an in-process dump showed the polyfill tracking one socket with_httpMessageDetachedunset and an active entry withheadersCompleted: false, req: null— a connection that never carried a request. CallingcloseIdleConnections()did not releaseclose();closeAllConnections()did.deno task verifyon this branch: every command green, Deno leg 520.5s against the 20m deadline (test:node 254.9s, test:bun 370.1s).Connection: closerequest header (verified empirically; Deno and Node close the socket immediately, and Bun'sclose()does not wait on connections).Scope
Included
Intentionally unchanged
useHttpDurableStreamkeeps pooled keep-alive fetch in production: serialized appends over one connection are the intended durable-execution behavior; connection reuse is incidental to this test, whose subject is gate rejection.DurableStreamTestServer.stop()(vendored@durable-streams/server) is not patched; itsclose()-only shutdown and Deno's polyfill gap are candidates for upstream reports, noted on 🧪 Deno HTTP journal teardown intermittently stalls concurrent verification #407.