persistit: do not accept short writes when copying pages to the volume#269
Merged
vharseko merged 1 commit intoJul 11, 2026
Conversation
FileChannel#write may report partial progress instead of throwing - the original authors observed this empirically on disk-full (see the comment in JournalManager#flush), and the same reporting is possible for transfers aborted by a concurrent interrupt-driven channel close, the suspected mechanism behind the Windows-only integrity violations in TransactionTest2.transactionsWithInterrupts. VolumeStorageV2.writePage ignored the returned count, so a short write during copyback silently tore the page in the volume file: intact header, stale tail. cleanupForCopy then dropped the journal copy, the pool evicted the page, and a later read served the torn page without any error, resurrecting stale record versions - matching the silent balance corruption observed on Windows CI. - VolumeStorageV2.writePage(ByteBuffer, long): write until every byte is transferred, mirroring the read loop in readPage; fail with PersistitIOException when no progress is made - safe because the copier retains the page in the page map and retries. - Buffer.load(): turn the buffer-length and page-address Debug asserts into hard InvalidPageStructureException checks so stale or torn page content fails loudly instead of surfacing as silently wrong data. - ErrorInjectingFileChannel: add a one-shot short-write injection. - IOFailureTest#testVolumeShortWriteMustNotTearPage: deterministic reproduction; without the fix it fails with CorruptVolumeException "page=6 ... is before left edge". Fixes OpenIdentityPlatform#268
maximthomas
approved these changes
Jul 11, 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.
Summary
Fixes #268.
FileChannel#writemay report partial progress instead of throwing — the original persistit authors observed this empirically on disk-full (see the comment inJournalManager#flush), and the same reporting is possible for transfers aborted by a concurrent interrupt-driven channel close (CancelIoExon Windows), the suspected mechanism behind the Windows-only integrity violations inTransactionTest2.transactionsWithInterrupts.VolumeStorageV2.writePage(ByteBuffer, long)ignored the returned count — a single unchecked_channel.write()call, in contrast toreadPagedirectly above it, which loops until the page is fully read. A short write during copyback therefore silently tore the page in the volume file: intact header, stale tail. The failure chain to the observed silent corruption:cleanupForCopydrops the journal copy, making the torn volume page authoritative.Buffer.load()validated nothing beyond a type-byte range check (the page-address and buffer-length checks were disabledDebugasserts, and all checked fields live in the intact header).Commit/rollback bookkeeping was audited and is exception-safe (abort is marked before any I/O; commit's
finallyaborts on flush failure), which rules out the transaction paths and leaves torn page I/O as the identified silent-corruption vector.Changes
VolumeStorageV2.writePage(ByteBuffer, long): write until every byte is transferred, mirroring thereadPageloop; throwPersistitIOExceptionwhen no progress is made — safe, because the copier retains the page in the page map and retries the copy on the next cycle.Buffer.load(): turn the buffer-length and page-addressDebugasserts into hardInvalidPageStructureExceptionchecks, so stale or torn page content fails loudly instead of surfacing later as silently wrong data.ErrorInjectingFileChannel: add a one-shot short-write injection (injectShortWriteOnce) that transfers only the bytes below a given position and returns the partial count without throwing, mimicking an aborted transfer.IOFailureTest#testVolumeShortWriteMustNotTearPage: deterministic end-to-end reproduction — a one-shot short write mid-page duringcopyBackPages(), then pool invalidation and full read-back verification. Without the fix it fails withCorruptVolumeException: ... page=6 ... is before left edge; with the fix the write loop transparently completes the transfer and all records read back intact.Testing
Buffer.load()-sensitive classes (BufferTest,MVCCPruneBufferTest,RecoveryTest,CorruptVolumeTest,WarmupTest,BufferPoolTest,IntegrityCheckTest): all green.persistit/coresuite: 566 tests, 0 failures, 0 errors, 5 pre-existing skips.#258 (a Windows skip for
transactionsWithInterrupts) was closed in favor of this fix: the test keeps running on Windows, so CI there validates the fix on the affected platform; a residual failure would now surface as a loudInvalidPageStructureExceptionrather than a silent balance mismatch (tracked in #268).