From bc54e3e0366e42437404ca5f0f68dc1681b3a9be Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Sat, 11 Jul 2026 10:22:39 +0300 Subject: [PATCH] persistit: do not accept short writes when copying pages to the volume 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 #268 --- .../src/main/java/com/persistit/Buffer.java | 18 ++++++- .../java/com/persistit/VolumeStorageV2.java | 19 ++++++- .../persistit/ErrorInjectingFileChannel.java | 27 +++++++++- .../java/com/persistit/IOFailureTest.java | 53 +++++++++++++++++++ 4 files changed, 113 insertions(+), 4 deletions(-) diff --git a/persistit/core/src/main/java/com/persistit/Buffer.java b/persistit/core/src/main/java/com/persistit/Buffer.java index ba71b16452..963dece395 100644 --- a/persistit/core/src/main/java/com/persistit/Buffer.java +++ b/persistit/core/src/main/java/com/persistit/Buffer.java @@ -486,8 +486,22 @@ void load() throws InvalidPageStructureException { _slack = 0; _rightSibling = 0; } else { - Debug.$assert0.t(getByte(BUFFER_LENGTH_OFFSET) * 256 == _bufferSize); - Debug.$assert0.t(getLong(PAGE_ADDRESS_OFFSET) == _page); + /* + * Hard checks, not Debug asserts: a page whose stored length + * or address does not match holds stale or torn content + * (e.g. from a partially transferred write) and must fail + * loudly here rather than surface later as silently wrong + * data. + */ + if (getByte(BUFFER_LENGTH_OFFSET) * 256 != _bufferSize) { + throw new InvalidPageStructureException("Invalid buffer length " + + (getByte(BUFFER_LENGTH_OFFSET) * 256) + " in content of page " + _page + ": expected " + + _bufferSize); + } + if (getLong(PAGE_ADDRESS_OFFSET) != _page) { + throw new InvalidPageStructureException("Invalid page address " + getLong(PAGE_ADDRESS_OFFSET) + + " in content of page " + _page); + } _alloc = getChar(FREE_OFFSET); _slack = getChar(SLACK_OFFSET); _rightSibling = getLong(RIGHT_SIBLING_OFFSET); diff --git a/persistit/core/src/main/java/com/persistit/VolumeStorageV2.java b/persistit/core/src/main/java/com/persistit/VolumeStorageV2.java index a09c50e705..8b4fe32699 100644 --- a/persistit/core/src/main/java/com/persistit/VolumeStorageV2.java +++ b/persistit/core/src/main/java/com/persistit/VolumeStorageV2.java @@ -507,7 +507,24 @@ void writePage(final ByteBuffer bb, final long page) throws PersistitIOException } try { - _channel.write(bb, page * _volume.getStructure().getPageSize()); + /* + * FileChannel#write may report partial progress instead of + * throwing - observed empirically on disk-full, and possible for + * transfers aborted by a concurrent interrupt-driven channel + * close. Accepting a short count here would silently tear the + * page in the volume file, so write until every byte has been + * transferred, mirroring the read loop in readPage. + */ + final int size = bb.remaining(); + final long position = page * _volume.getStructure().getPageSize(); + int written = 0; + while (written < size) { + final int bytesWritten = _channel.write(bb, position + written); + if (bytesWritten <= 0) { + throw new IOException("Unable to write bytes at position " + (position + written) + " in " + this); + } + written += bytesWritten; + } } catch (final IOException ioe) { _persistit.getAlertMonitor().post( new Event(AlertLevel.ERROR, _persistit.getLogBase().writeException, ioe, _volume, page), diff --git a/persistit/core/src/test/java/com/persistit/ErrorInjectingFileChannel.java b/persistit/core/src/test/java/com/persistit/ErrorInjectingFileChannel.java index 82001e0609..e8efaef0b1 100644 --- a/persistit/core/src/test/java/com/persistit/ErrorInjectingFileChannel.java +++ b/persistit/core/src/test/java/com/persistit/ErrorInjectingFileChannel.java @@ -12,6 +12,7 @@ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. + * Portions Copyrighted 2026 3A Systems, LLC */ package com.persistit; @@ -44,6 +45,7 @@ class ErrorInjectingFileChannel extends FileChannel implements TestChannelInject volatile IOException _injectedIOException; volatile String _injectedIOExceptionFlags; volatile long _injectedDiskFullLimit = Long.MAX_VALUE; + volatile long _injectedShortWritePosition = -1; @Override public void setChannel(final FileChannel channel) { @@ -87,13 +89,27 @@ void injectTestIOException(final IOException exception, final String flags) { /** * Sets a file position at which writes will simulate a disk-full condition * by throwing an IOException. - * + * * @param limit */ void injectDiskFullLimit(final long limit) { _injectedDiskFullLimit = limit; } + /** + * Arms a one-shot short write: the next write that spans the supplied + * position transfers only the bytes below it and returns the partial + * count without throwing. This mimics a transfer aborted mid-flight + * (e.g. by a concurrent close of the channel on Windows) and the + * empirically observed disk-full behavior of FileChannel#write, both of + * which report partial progress instead of failing. + * + * @param position + */ + void injectShortWriteOnce(final long position) { + _injectedShortWritePosition = position; + } + @Override protected void implCloseChannel() throws IOException { _channel.close(); @@ -145,6 +161,15 @@ public int write(final ByteBuffer byteBuffer, final long position) throws IOExce if (byteBuffer.remaining() == 1) { injectFailure('e'); } + final long shortWriteAt = _injectedShortWritePosition; + if (shortWriteAt >= 0 && position <= shortWriteAt && position + byteBuffer.remaining() > shortWriteAt) { + _injectedShortWritePosition = -1; + final int delta = (int) (position + byteBuffer.remaining() - shortWriteAt); + byteBuffer.limit(byteBuffer.limit() - delta); + final int written = byteBuffer.hasRemaining() ? _channel.write(byteBuffer, position) : 0; + byteBuffer.limit(byteBuffer.limit() + delta); + return written; + } final long capacity = Math.max(0L, _injectedDiskFullLimit - position); final int delta = (int) Math.max(0L, byteBuffer.remaining() - capacity); byteBuffer.limit(byteBuffer.limit() - delta); diff --git a/persistit/core/src/test/java/com/persistit/IOFailureTest.java b/persistit/core/src/test/java/com/persistit/IOFailureTest.java index a5cc4100e4..1e06394e27 100644 --- a/persistit/core/src/test/java/com/persistit/IOFailureTest.java +++ b/persistit/core/src/test/java/com/persistit/IOFailureTest.java @@ -12,6 +12,7 @@ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. + * Portions Copyrighted 2026 3A Systems, LLC */ package com.persistit; @@ -280,6 +281,43 @@ public void run() { volume.getPool().invalidate(volume); } + /** + * Issue #268. FileChannel#write may report partial progress instead of + * throwing - observed empirically on disk-full, and suspected for + * transfers aborted by a concurrent interrupt-driven channel close on + * Windows. VolumeStorageV2.writePage used to ignore the returned count, + * so a short write during copyback silently tore the page in the volume + * file: intact header, stale tail. The journal copy was then dropped by + * cleanupForCopy, the pool evicted the page, and a later read served the + * torn page with no error - resurrecting stale record versions. + */ + @Test + public void testVolumeShortWriteMustNotTearPage() throws Exception { + store1(0); + final Volume volume = _persistit.getVolume(_volumeName); + final ErrorInjectingFileChannel eifc = errorInjectingChannel(volume.getStorage().getChannel()); + /* + * The next volume write that spans the middle of page 6 - a tree page + * after store1 - transfers only half the page and returns the short + * count without an exception, exactly once. Subsequent writes are + * normal, so the copyback cycle completes and cleanupForCopy drops the + * journal copies, making the volume file authoritative. + */ + final int pageSize = volume.getPageSize(); + eifc.injectShortWriteOnce(6L * pageSize + pageSize / 2); + _persistit.copyBackPages(); + /* + * Force every subsequent read to come from the volume file. + */ + volume.getPool().flush(Long.MAX_VALUE); + _persistit.getJournalManager().force(); + volume.getPool().invalidate(volume); + /* + * All records must read back intact through the volume channel. + */ + checkStore1(0); + } + @Test public void testJournalEOFonRecovery() throws Exception { final JournalManager jman = _persistit.getJournalManager(); @@ -465,6 +503,21 @@ private void store1(final int at) throws PersistitException { _persistit.releaseExchange(exchange); } + private void checkStore1(final int at) throws PersistitException { + final Exchange exchange = _persistit.getExchange(_volumeName, "IOFailureTest", false); + final StringBuilder sb = new StringBuilder(); + + for (int i = 1; i <= 5000; i++) { + sb.setLength(0); + sb.append((char) (i / 20 + 64)); + sb.append((char) (i % 20 + 64)); + exchange.clear().append(at).append(sb).fetch(); + assertEquals("Record " + at + "_" + i + " should read back intact", "Record #" + at + "_" + i, exchange + .getValue().isDefined() ? exchange.getValue().getString() : null); + } + _persistit.releaseExchange(exchange); + } + private void copyBackEventuallySucceeds(final long start, final String reason) throws Exception { final long expires = System.currentTimeMillis() + 10000; boolean done = false;