Repository navigation
Fix fetch cursor wedging permanently after a seek/decode error - #20
Merged
Merged
Conversation
Execute() now resets the cursor on error and ResetCursor() resets unconditionally (via tryReset()); previously cursor_pageno was zeroed without resetting the stepped statement, so the job's next bind failed with SQLITE_MISUSE until reopen.
Second-opinion review cycle on the F3 diff (see PR body): the fix itself was confirmed, but it shipped with no committed regression coverage. test/test.cc now carries both wedge shapes (corrupt page; missing root page with cursor_pageno == 0), mutation-verified three ways; test_exe links zstd and compiles zstd_vfs.h so the tests drive the real ZstdVFS. The ResetCursor rationale now lives in one place.
mlin
marked this pull request as ready for review
October 25, 2026 06:49
Wid-L-Hacker
added a commit
to Wid-L-Hacker/sqlite_zstd_vfs
that referenced
this pull request
Oct 25, 2026
mlin
pushed a commit
that referenced
this pull request
Oct 25, 2026
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.
A seek or decode error can leave a fetch job's cursor stepped while its bookkeeping says "unpositioned". Later reads assigned to that job then fail internally with SQLITE_MISUSE until the database file is closed and reopened.
What happens today
Execute()catches the exception and setscursor_pageno = 0, but it does not reset the underlying statement.ResetCursor()also skips its reset whencursor_pagenois already 0. That combination breaks an invariant: a "missing page" seek can leave the cursor stepped onto a neighbouring page whilecursor_pagenois still 0. Socursor_pageno == 0no longer guarantees "unpositioned and safe to bind", and the next attempt to bind that statement fails with SQLITE_MISUSE.The fix
Execute()'s error path now callsResetCursor()instead of only zeroingcursor_pageno.ResetCursor()resets unconditionally, viatryReset().sqlite3_reset()resets the statement even when it returns the preceding execution's error code;tryReset()discards that already-handled status, which is what the destructor and thenoexceptcallers ofResetCursor()need.cursor_pageno + 1 == pageno) does not callResetCursor(), is unchanged, and gains no resets.How it was verified
Points for review
This PR is a draft because the fix touches cursor state handling, and three things deserve a maintainer's eye:
cursor_pageno == 0, and any borrowed page data discarded before the job publishes its result.cursor_pagenowas never set. That case is the reason the old> 0gate had to go.Related: the ThreadPool races found alongside this (C1/C2 in the audit) live in sqlite_web_vfs and reach this repo through its web pin, not through this PR.
Found in the 2026-10-23 correctness audit of the GenomicSQLite family.