Do not set pMethods when xOpen fails (a failed open closes an unrelated live file) - #21
Open
jtarchie wants to merge 1 commit into
Open
Do not set pMethods when xOpen fails (a failed open closes an unrelated live file)#21jtarchie wants to merge 1 commit into
jtarchie wants to merge 1 commit into
Conversation
jtarchie
added a commit
to jtarchie/sqlitezstd
that referenced
this pull request
Sep 2, 2026
The 2026-09-01 Knowhere outage was not the temp-file bug: one failed open through the zstd VFS closed the file under the FIRST connection the process had opened, and that connection then failed every read its page cache could not answer -- as "SQL logic error" and "fts5: corruption found reading blob", against a database that was intact the whole time. Health checks kept passing because their pages were cached. The close happens in psanford/sqlite3vfs: sqlite calls xClose on a file whose xOpen failed whenever pMethods is set, s3vfsOpen set it unconditionally, and the unregistered file's zeroed id aliased file 0. Fixed upstream in psanford/sqlite3vfs#21; the replace directive moves to a commit carrying both #20 and #21. The new spec is the outage in miniature: query, fail to open a missing database, query a table the cache has not seen. Against the old dependency it fails with the outage's exact error; against #21 it passes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WMBg7JBvZJRRu1RytUj5c3
sqlite calls xClose on any file whose xOpen failed, as long as pMethods is set. s3vfsOpen set it unconditionally -- but a failed goVFSOpen never registered the file, so the s3vfsFile still carried its zero value, and the xClose that followed looked up id 0: the first file the process ever opened. That file was closed out from under the connection that owns it. The owning connection then failed every read that missed its page cache, while everything already cached kept answering. The errors said "SQL logic error" and "fts5: corruption found reading blob ..." -- nothing pointed at a close, a failed open of an unrelated file, or this glue. On a long-lived process serving a large read-only database, that is a persistent outage with a misleading name: observed in production on 2026-09-01, where one bad open broke the serving connection for hours and the database was blamed for corruption it did not have. Leave pMethods NULL on failure, which is sqlite's documented way of saying there is nothing to close. Start file ids at 1 as well, so a zeroed s3vfsFile can never alias a live file through any other path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WMBg7JBvZJRRu1RytUj5c3
jtarchie
force-pushed
the
fix-failed-open-close
branch
from
September 2, 2026 02:57
145730f to
5e04426
Compare
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.
Problem
If opening one database fails, a different, healthy connection in the same process silently breaks.
Why: SQLite calls
xCloseon a failed open wheneverpMethodsis set, ands3vfsOpensets it even on failure. But a failed open never registered the file, so its id is still 0 — and id 0 belongs to the first file the process opened. ThatxClosecloses the healthy file out from under its owner.The broken connection keeps answering from its page cache and fails every other read, with errors that blame the database instead (
SQL logic error,fts5: corruption found reading blob ...). This took down our production server for hours on an intact database.Fix
pMethodsonly when the open succeeds; leave it NULL on failure (SQLite's way of saying "nothing to close").Test
TestFailedOpenDoesNotCloseAnotherFileopens a database, fails to open a missing one, then checks the first is still usable. Before the fix it fails withfile already closed; after, it passes.(Independent of #20.)