Fix IDBBatchAtomicVFS writes that do not match the stored blocks - #351
Open
lalexdotcom wants to merge 2 commits into
Open
lalexdotcom wants to merge 2 commits into
lalexdotcom wants to merge 2 commits into
Conversation
jWrite's overwrite branch fetched a single block and wrote the whole buffer through it, assuming a block starts exactly at iOffset and holds at least as much as is being written. Neither holds in general. Nothing may start at iOffset. The range then yields the next key, which is the PREVIOUS block in the file, and subarray(iOffset + block.offset) lands past its end: a RangeError on Chromium, "source array is too long" on Firefox, and on the asyncify build the rejection is swallowed and the call never settles. SQLite reaches this by writing a transient file out of order, which it does when a statement is interrupted while materialising a result. A block may also be shorter than the write. SQLite lays down a 512-byte journal header and later writes a full page at the same offset; the write then runs past the end of the block that covers its start, with the same result. jRead already walks the blocks it finds and checks that each one reaches the offset asked for. jWrite now does the same: it writes into each block that covers part of the range, and stores what no block covers as a new block, which is what the branch above it already does for a file being extended. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two cases, both through jWrite and jRead directly, so neither depends on how SQLite happens to order its writes. A gap: two blocks are written with a hole between them, then the hole is filled. Against the previous behaviour the fill lands past the end of the block before it, and every later read of the file returns SQLITE_IOERR_READ and zeroes - a silent corruption, with no error raised at the write. A short block: 512 bytes are written, then a full page over them. The write runs past the end of the block that covers its start. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This branch has not been deployed
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.
What happens
jWrite's overwrite branch fetches one block and writes the whole buffer through it:That assumes a block starts exactly at
iOffsetand holds at least as much as is being written. Neither is guaranteed, and both fail in practice.Nothing may start at
iOffset. The keys are-offset, soIDBKeyRange.bound([path, -iOffset], [path, Infinity])yields the block with the greatest offset ≤iOffset— the block before the one wanted, when nothing starts there.subarray(iOffset + block.offset)then lands past its end. SQLite reaches this by writing a transient file out of order, which it does when a statement is interrupted while materialising a result: measured on such a file, the write at 1843200 found the block at 1839104, a single 4096-byte page missing from an otherwise contiguous run of 2305 blocks.A block may be shorter than the write. SQLite writes a 512-byte journal header, then a full page at the same offset. The write runs past the end of the block that covers its start.
The symptom depends on the build:
offset is out of boundson Chromium,source array is too longon Firefox, and on the asyncify build the rejection is swallowed and the call never settles. When the failure happens inside the queued IndexedDB operation rather than at the call, the file is left in a state where every later read returnsSQLITE_IOERR_READand zeroes — corruption with nothing raised at the write.The change
jReadalready walks the blocks it finds, and already checks that each one reaches the offset asked for:jWritenow does the same walk with the same test. It writes into each block that covers part of the range, and stores what no block covers as a new block — which is what the branch directly above it already does for a file being extended. The asymmetry between the two was the defect: the reader knew the blocks could be of any size and arranged with gaps, the writer did not.Tests
test/vfs_sparse_write.js, wired intotest/IDBBatchAtomicVFS.test.js. Both cases go throughjWriteandjReaddirectly, so neither depends on how SQLite happens to order its writes:masterevery later read of the file returnsSQLITE_IOERR_READand zeroes.masterthe write runs past the end of the block.With the change, the file's 48 tests pass, and the full suite is 2910 passing on Chromium.
Checklist
non-exclusive, royalty-free, irrevocable copyright license to reproduce, prepare
derivative works of, publicly display, sublicense, and distribute this
Contribution and such derivative works.
Contribution contains no content requiring a license from any third party.