Repository navigation
Event contract corpus; fix the pause crash, #480 and #481 (v6 groundwork) - #482
Merged
Merged
Conversation
Adds test/contract: scenarios whose exact sequence of events (submitted, meta, row, column, info, error, done, free, callbacks, cancel callbacks) is pinned in corpus/ as reviewable transcripts, with capture.js to record and compare them (including against another build) and contract.test.js to enforce them. Each run is in its own process, so hangs, runaways and crashes stay contained. The corpus currently pins what 5.6.0 does, bugs included (status: unreviewed), covering #478 - #481 plus a crash found on pause/resume every row. The issue repros are in test/contract/repro and the findings in test/contract/HANDOVER.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… once A pause on the last row of a batch was ignored: dispatch() went on to the next result set (or ended the query) while paused, and resume() then did the same step again from the stale batch. Two nextResultSet calls in flight on one statement crashed the process (0xC0000005, pause/every-row about 1 run in 3). Otherwise the second call skipped a result set (#480), or after the last row done/free came twice followed by an endless rowcount 0 loop. A query now has one next step at a time: running, or parked by a pause. resume() runs the parked step and never starts another. Nothing, done included, is emitted while paused. The three pause transcripts are now agreed; the pause test that expected done while still paused now resumes first. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ry holds the connection (#481) The queue advanced by removing whatever was at its head, so a query that failed advanced once too often: work queued behind it was removed just after it started, two statements overlapped ("Connection is busy with results for another command"), and a later query could no longer be cancelled. - WorkQueue.finish(op) removes that item only, by identity, starts the next only if it was the head, and is a no-op the second time. nextOp() is gone. - Each query keeps its own queue item. A prepared statement shares one notify across its executions, so the notify's operation was only the latest one. - A statement's unbind and free run straight after their query, before work already waiting (enqueueFollowUp), so 'free' is no longer delayed behind it. - close() always finishes its item: the cancelled branch of moveToNextResult cleared 'running' first, which is what the extra advance in end() covered. - dropItem matches by identity, not priority. A paused query now holds the connection. It keeps its place at the head, rather than moving to the back and removing the item waiting behind it without running it (that query was never submitted, and its callback never fired). A query submitted while another is paused waits for it, rather than freeing it. Cancelling a paused query ends it as any cancel does (HY008, then free), rather than freeing it silently, and close() cancels a paused query first. Corpus: the queue and paused-cancel transcripts are re-recorded and now deterministic; queue/ok-then-two-on-* are agreed. pause.test.js and querycancel.test.js are changed to the new rules. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A paused query holds the connection. Rather than queue new work behind it
indefinitely, a query submitted while another is paused is refused at once:
its callback (or 'error') gets an error with sqlstate HY010 (function sequence
error), then 'done' and 'free', and no 'submitted'. beginTransaction, commit,
rollback and prepare get the error in their callback. Work queued before the
pause still waits its turn.
Once a cancel of a paused query is accepted, the query no longer holds the
connection, so work submitted from the cancel callback queues behind it.
Corpus: queue/paused-then-{submit,cancel-queued-before,resume-queued-before};
the submit and resume cases are agreed.
The Linux CI runs showed 4 error scenarios that differ from Windows: a batch
whose first statement fails at execute loses the rest of the batch on Linux
(more=false, no done). The base commit does the same, so it is older than
this branch (5.6.0's #475 look-ahead). Pinned as .linux.txt transcripts with
status known-bug, captured on WSL Fedora, and described in HANDOVER.md.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Open
7 tasks
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
1 participant
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.
Groundwork for v6. It is not for release yet. The issues are referenced but not closed: they should close when a release carries the fix.
What's here
An event contract corpus (
test/contract/): 77 scenarios whose exact event sequence (submitted,meta,row,info,error,done,free, callbacks, cancel callbacks) is pinned incorpus/as reviewable transcripts. It covers #478 – #481. The findings and the plan are intest/contract/HANDOVER.md.Fixed: pause/resume crash (0xC0000005), #480, and the endless
rowcount 0loop. One cause: a pause on the last row of a batch was ignored, andresume()then took the same step again. TwonextResultSetcalls were in flight on one statement (crash), or a result set was skipped (#480). A query now has one next step at a time: running, or parked by a pause.Fixed: #481, the work queue.
WorkQueue.finish(op)replaces the blindnextOp(): each owner finishes its own item, once, by identity. A statement's unbind and free run straight after their query. Each query keeps its own queue item: a prepared statement shares one notify across its executions.A paused query holds the connection. Work queued before the pause waits its turn. Work submitted while a query is paused is refused:
HY010, thendoneandfree(nosubmitted). Transaction calls andprepareget the error in their callback. A newly submitted query no longer frees the paused one. Cancelling a paused query ends it with HY008 →free, andclose()cancels a paused query first. This also fixes a 5.6.0 bug: a query queued before another one paused was silently dropped, never submitted, and its callback never called.Behaviour changes (v6)
doneincluded.HY010, instead of freeing the paused one. The same applies to transaction calls andprepare.errorHY008, as any cancel does. Before, it was freed silently.pause.test.jsandquerycancel.test.jsare rewritten to these rules.Still to do (see HANDOVER.md)
doneon every query, exactly once, thenfree: missing on every error path today, and too early on cancel (5.6.0: on cancel, 'done' is emitted before the query has ended, and also when nothing was cancelled #479).Testing
pause/every-row: 23 identical runs, no crash. Before: about 1 run in 3 crashed.errors/*scenarios only. A batch whose first statement fails at execute loses the rest of the batch on Linux (more=false, nodone). That is reproduced on WSL Fedora, and it happens with the base commit'slib/too, so it predates this PR: 5.6.0's fix: prints, errors and results after a failed statement in a batch were lost #475 look-ahead does not hold on Linux. It is pinned as.linux.txttranscripts (# status: known-bug) so CI stays green, and it needs its own fix.🤖 Generated with Claude Code