Repository navigation
test: fix the three Windows CI races (C6 multipart reset, store lock wait, smoke keep-alive reuse) - #169
Merged
Conversation
…buffer, so C6's multipart leaves no tail unread C6 failed the windows (node 22) job of run 38737724941 with `fetch failed` at the serverFetch call in same(), on the tree that had passed at 7950b03. Its multipart case POSTs a FormData, which fetch streams in parts: the headers and the first part in one write, the closing boundary in a later one. The server parses no multipart, so retrofeed answers 404 from the headers alone, and with connection: close Node's server closes right after the answer with the closing boundary unread, so its TCP stack sends an RST. Linux still hands the client the 404; Windows discards what the client had not yet read when the RST lands (WSAECONNRESET), and fetch, with no status line yet, rejects. serverFetch (the same text in test-bundle-compare-pairs.mjs, which C3 pins) builds the request with new Request() and gives fetch its body as one ArrayBuffer, so the body leaves in the headers' write and the server reads all of it before it closes. The bytes and the Content-Type, boundary included, are what fetch would send; the shim still gets the original FormData; a GET sends no body; connection: close stays. On Linux (node 22.22.2), 40 multipart requests to a real child server made its kernel send 26-29 RSTs before and 0 after. Through a relay that drops unread data on an RST the way Winsock does, fetch failed with ECONNRESET 1-5 times in 40 before and 0 after. Under four busy loops, ten runs of the 40 gave 6-16 RSTs and 3-10 failures in each run before, and 0 and 0 in each run after. C3 now pins it: a FormData goes to fetch as one buffer holding the bytes fetch serializes it to, with connection: close, and a GET has a null body. The pin fails against the previous serverFetch. It is an assertion in an existing test, so the suite total stays at 1519; the CHANGELOG entry quotes it as 1519 → 1519. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UhPGnqVJ24goSVtbKpdkLJ
…ts, and the child syncs nothing while it holds the lock "Concurrency: a migration waits out a child holding BEGIN IMMEDIATE, then applies" failed the windows (node 22) job of run 38737678276 with "database is locked" after 9.9 s. In the same run the write sibling waited 2355 ms of a 700 ms hold. Each Concurrency test races two clocks. The parent's busy_timeout starts at 5000 ms when the child prints "locked". The child's window is its 700 ms hold, plus its COMMIT, plus however long a loaded runner keeps it off the CPU. The migration's child pays the most: closeStore removed the -wal, and openRaw leaves synchronous at FULL, so inside the window its commit syncs a new WAL header and then the commit. When the window outlasts the budget, SQLite's busy handler gives up and BEGIN IMMEDIATE fails. - lockHolder sets synchronous=OFF, so the child makes no fsync between "locked" and its exit. The child's durability is not under test; its lock is. The backup test that uses lockHolder(2500) checks only that the uncommitted row is absent. - Every parent that waits a holder out (the write, the migration and the repository write) has a busy_timeout of LOCK_WAIT_LIVENESS_MS, 30 s. That is a liveness bound and nothing asserts it: the tests assert only waited >= 300 ms and what was written. A successful wait still ends at the child's COMMIT, about 700 ms. BUSY_TIMEOUT_MS (5000) is unchanged and still pinned on openStore. Reproduced on Linux (node 22.22.2), ten runs at base and ten with the fix, with every fsync delayed by 1.8 s (strace -f -e inject=fsync:delay_exit=1800000): - Base: the write and repository write waited 4.2-4.3 s and passed, and the migration failed with "database is locked" in every run, as on CI. - Fix: all three passed in every run; the write and repository write waited 0.6-0.7 s. With the child's hold raised to 6 s, a window past the 5000 ms budget as a stalled child's would be, all three tests failed at base and pass with the fix. No test is added or removed; the suite total stays at 1519. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UhPGnqVJ24goSVtbKpdkLJ
… the Concurrency tests do "Migrations: two openers racing the same step apply it once" ran the race 378fb77 took out of the three Concurrency tests. Its child's runMigrations takes BEGIN IMMEDIATE, says "locked" from inside the step, holds 700 ms and commits. The file is fresh, so it is in rollback-journal mode, and openRaw leaves synchronous at FULL: the journal and the database are fsynced inside the window. The parent's openRaw waited at the store's 5000 ms busy_timeout. - The child sets synchronous=OFF right after openRaw. Its durability is not under test; the parent re-reading user_version inside its transaction is. - The parent opens with timeout LOCK_WAIT_LIVENESS_MS (30 s), a liveness bound nothing asserts. Every assertion stands: the child applied [1], the parent [], and the applied table holds only "child". Reproduced on Linux (node 22.22.2) with every fsync delayed by 1.8 s (strace -f -e inject=fsync:delay_exit=1800000), ten runs each, beside the three Concurrency tests: - At 378fb77: "database is locked" in 10 of 10 runs; the three Concurrency tests passed beside it. - With the fix: all four passed in 10 of 10 runs. With the child's hold raised to 6 s and no fsync delay: "database is locked" after 5.1 s at 378fb77, a pass with the fix. The sweep of the server/ and tools/ tests for a parent that waits on a lock another process holds (lockHolder, BEGIN IMMEDIATE, Atomics.wait, openRaw beside a spawned holder) finds these four and no others: - "packc store backup while a server holds the store" runs beside lockHolder(2500). Its VACUUM INTO only reads a WAL database and never waits on the writer, and it passes under the same fsync delay. - The restore tests expect the in-use refusal, with no busy wait. - test-authz, test-auth-proxy and test-transport-hook plant rows with openRaw into a running server's store while the server is idle, so the server holds no lock. No test is added or removed; the suite total stays at 1519. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UhPGnqVJ24goSVtbKpdkLJ
…uest rides a socket both keep-alive timers have expired
The smoke suite failed the windows (node 22) job of run 38737724941 (node
22.16.0) at server/test-smoke.mjs:1578. That line is the POST
/api/refresh-live with no mcpUrl, which expects a 400. It failed with
"TypeError: fetch failed", cause "read ECONNRESET".
Just before it, the boot-line check runs three boot()s. Each is a
spawnSync of a whole server start, so it stops this process's event loop
for the whole boot. That loop also runs the suite's own server, start()ed
in the same process at Node's default keepAliveTimeout (5 s; an idle
socket closes about 6 s after its last answer). fetch's socket to it,
pooled since the request before the check, sits idle through the boots.
It goes past fetch's keep-alive (the advertised 5 s less 2 s) and past
the server's, but neither timer can run.
The POST takes that socket at once. When the loop resumes in its poll,
check or close phase, the server's timer runs before the server reads the
socket and closes it with the request unread. The kernel answers with a
reset, and fetch retries nothing on a socket error. Node 22's undici
(6.21.2, 6.24.1) has no idle-socket check before reuse; undici 7 does.
- server/fixtures/serve-child.mjs exports bootAsync: the same child,
environment and result as boot(), through spawn. boot() is unchanged
for its other callers.
- The smoke awaits bootAsync for its three boots. Both timers run while
the children boot, and the POST opens a new connection.
- In T8 of tools/test-studio-bundle.mjs, the two requests to the serve()
child go out with "Connection: close", as serverFetch does in
tools/test-bundle-compare.mjs. A sweep of npm test (below) found this
one; it did not fail in 10 runs at the base.
Reproduced on Linux, with every boot child delayed 2 s by a --require
preload (a slow runner's boot), running node server/test-smoke.mjs:
- node 22.22.2, base: the last two batches (30 and 10 runs) failed in
every run with "fetch failed" / "read ECONNRESET" at
test-smoke.mjs:1578, as on CI. Two earlier batches of 10 had passed,
because whether the reset comes depends on the phase the loop resumes
in.
- node 22.22.2, fix: 50 of 50 passed.
- node 22.16.0: 3 of 3 failed at the base and 3 of 3 passed with the fix.
A reduced model has an in-process server, one pooled request, the loop
blocked 6 s by spawnSync, then a POST. It gets the reset in every run when
the test resumes in a check or poll phase, and in none when it resumes
inside a timer callback. With the block as an awaited spawn, 0 of 15 runs
fail.
The sweep ran the whole npm test at the base with every fetch and
node:http request timed against the event loop's last timer tick. It
flagged three requests issued at least 0.7 s after a synchronous block,
on an origin already used:
- test-smoke.mjs:1578, fixed here.
- The serverFetch of tools/test-bundle-compare.mjs, which already sends
"connection: close".
- T8's GET /api/taxonomy, after two spawnSync bundle builds (2.0 s under
the suite's load) with the registration's socket pooled. With each
build delayed 3 s (8 s idle), T8 passed 10 of 10 at the base and 10 of
10 with the fix. Its pooled socket had served one request, and in the
model such a socket's next request was answered, while one that had
served two failed ("other side closed") in alternate runs.
No test is added or removed; the suite total stays at 1519.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UhPGnqVJ24goSVtbKpdkLJ
…aw, the smoke job's name, and which undici has the idle-socket check The section's heading presented every fix as a race the Windows leg lost; the migration two-openers test and T8 were found by the sweep beside them. The smoke failure was the windows (node 22.16.0) job, not windows (node 22). undici's idle-socket check (kIdleSocketValidation) first shipped in 7.28.0 and 8.5.0, not in every undici 7; 18d726a's message says "undici 7". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UhPGnqVJ24goSVtbKpdkLJ
MoebiusX
marked this pull request as ready for review
October 15, 2026 17:37
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.
Fixes the three races that turned the Windows CI legs red on 93586a9, the 0.6.0 cut. That tree had passed 7/7 at 7950b03, and each failing job passed on its single re-run. A sweep for each race class then fixed the same race in two places beside them. These are test fixes only: no runtime,
tools/lib, golden or output change, and the suite total stays 1519 (no test is added or removed).The races
fetch failed(windows (node 22),tools/test-bundle-compare.mjs:1109). C6's multipart POST streams its closing boundary in a separate write. The server answers its 404 from the headers alone. Withconnection: closeset, Node closes the socket with that tail still unread, so the server's TCP stack sends a reset. Windows then discards the 404 the client hadn't read yet (WSAECONNRESET). Linux delivers it.serverFetchhands fetch the body as one buffer, with the same bytes and Content-Type.serverFetchsends a body this way, and the check fails against the oldserverFetch.fetch failedper 40 before and 0 after.database is locked(windows (node 22),server/test-store.mjs:592). The lock-holding child holds 700 ms plus its COMMIT's sync. On a slow runner that outlasts the parent's 5 s busy timeout.synchronous=OFF. The parents that wait a holder out wait up toLOCK_WAIT_LIVENESS_MS(30 s). That bound is liveness only; no assertion reads it.openStore.read ECONNRESET(windows (node 22.16.0),server/test-smoke.mjs:1578). The boot-line check runs threespawnSyncboots. They freeze the event loop that both the client and the in-process server run on. The POST then goes out on a pooled socket both keep-alive timers have expired, and the server closes it unread.serve-child.mjsgainsbootAsync, the same child, environment and result throughspawn. The smoke test awaits it for those three boots.tools/test-studio-bundle.mjs(two requests afterspawnSyncbuilds) now sendsConnection: close. That one is preventive; it never failed.Commits
serverFetchsends the body in one buffer.bootAsync, plus T8.Gate (HEAD 18d726a; ab78c97 changes only the CHANGELOG wording, re-checked with doc-totals 6/6 and the manifest in sync)
npm test: 1520 tests, 1519 pass, 1 environmental skip, on Node 22.22 and 22.16 with Playwright.vendor-manifest:checkin sync (57);sync-spec --checkclean; doc-totals and platform 6/6 each.Sweep (no other instance)
serverFetchalready sendsconnection: close.🤖 Generated with Claude Code
https://claude.ai/code/session_01UhPGnqVJ24goSVtbKpdkLJ
Generated by Claude Code