From 1b95688588d39294a5f1b3af62396ab922d008e3 Mon Sep 17 00:00:00 2001 From: Arun Sharma Date: Fri, 6 Nov 2026 08:20:18 -0800 Subject: [PATCH] fix(storage): throw instead of crashing on torn WAL tail reads BufferedFileReader::read copied the full requested size after a page refill without checking that the final page held that many bytes, so a WAL truncated mid-record (ENOSPC, power loss, SIGKILL mid-commit) overran the heap buffer and crashed recovery with SIGSEGV/SIGBUS in both strict and lenient modes. Loop small reads across page boundaries and pre-validate large reads against the file size, so torn tails surface as catchable replay errors: strict mode throws and lenient mode drops the torn transaction. Add WalTest.TornWALTailInsideLargeStringRecord covering a mid-string truncation of a multi-page WAL in both modes. Also fix a pre-existing uintmax_t/uint64_t deduction error in checkpoint_test.cpp that blocked building the transaction tests. --- src/common/serializer/buffered_file.cpp | 34 ++++++++++++++------- test/transaction/checkpoint_test.cpp | 2 +- test/transaction/wal_test.cpp | 39 +++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 12 deletions(-) diff --git a/src/common/serializer/buffered_file.cpp b/src/common/serializer/buffered_file.cpp index 47a1d7c66c..624a3a8cd9 100644 --- a/src/common/serializer/buffered_file.cpp +++ b/src/common/serializer/buffered_file.cpp @@ -1,5 +1,6 @@ #include "common/serializer/buffered_file.h" +#include #include #include "common/assert.h" @@ -78,20 +79,31 @@ void BufferedFileReader::read(uint8_t* data, uint64_t size) { // Clear read buffer. fileOffset -= bufferSize; fileOffset += bufferOffset; + bufferOffset = 0; + bufferSize = 0; + // readFromFile silently short-reads at EOF, so check the range first and throw a + // catchable error instead of returning uninitialized bytes. A torn WAL tail (e.g. a + // crash or ENOSPC mid-commit) must surface as a replay failure, not a native crash. + if (fileOffset > fileSize || size > fileSize - fileOffset) { + throw RuntimeException( + std::format("Reading past the end of the file {} with size {} at offset {}", + fileInfo.path, fileSize, fileOffset)); + } fileInfo.readFromFile(data, size, fileOffset); fileOffset += size; - bufferOffset = bufferSize; - } else if (bufferOffset + size <= bufferSize) { - memcpy(data, &buffer[bufferOffset], size); - bufferOffset += size; } else { - auto toCopy = bufferSize - bufferOffset; - memcpy(data, &buffer[bufferOffset], toCopy); - bufferOffset += toCopy; - readNextPage(); - auto remaining = size - toCopy; - memcpy(data + toCopy, buffer.get(), remaining); - bufferOffset += remaining; + // Loop across page boundaries: the final page of a torn file may hold fewer bytes + // than requested, and copying the full remainder would overrun the heap buffer. + while (size > 0) { + if (bufferOffset >= bufferSize) { + readNextPage(); + } + const auto toCopy = std::min(size, bufferSize - bufferOffset); + memcpy(data, &buffer[bufferOffset], toCopy); + bufferOffset += toCopy; + data += toCopy; + size -= toCopy; + } } } diff --git a/test/transaction/checkpoint_test.cpp b/test/transaction/checkpoint_test.cpp index dc8f96e0b3..84ad2aacff 100644 --- a/test/transaction/checkpoint_test.cpp +++ b/test/transaction/checkpoint_test.cpp @@ -1927,7 +1927,7 @@ TEST_F(CheckpointRetryAfterFailureTest, StrayAbortRecordAfterCheckpointIsIgnored conn.reset(); database.reset(); std::ofstream wal{walPath, std::ios::binary | std::ios::app}; - for (const auto start : {uint64_t{1}, blockBeforeLastCheckpoint}) { + for (const auto start : {uint64_t{1}, static_cast(blockBeforeLastCheckpoint)}) { const auto record = serializeCheckpointRecord(AbortRecord{start}); wal.write(reinterpret_cast(record.data.get()), record.size); if (enableChecksums) { diff --git a/test/transaction/wal_test.cpp b/test/transaction/wal_test.cpp index 6351b71040..4c37eb4351 100644 --- a/test/transaction/wal_test.cpp +++ b/test/transaction/wal_test.cpp @@ -1127,6 +1127,45 @@ TEST_F(WalTest, CorruptedWALTailTruncated2) { ASSERT_EQ(res->getNumTuples(), 3); } +// A torn WAL tail inside a multi-page string record (ENOSPC/power loss mid-commit) must not +// crash the replayer: BufferedFileReader reads cross a page boundary with a short final page. +// Non-strict recovery drops the torn transaction; strict recovery throws a catchable error. +TEST_F(WalTest, TornWALTailInsideLargeStringRecord) { + if (inMemMode || systemConfig->checkpointThreshold == 0) { + GTEST_SKIP(); + } + ASSERT_TRUE(conn->query("CALL force_checkpoint_on_close=false")->isSuccess()); + ASSERT_TRUE(conn->query("CALL auto_checkpoint=false")->isSuccess()); + ASSERT_TRUE( + conn->query("CREATE NODE TABLE Blob(id INT64 PRIMARY KEY, s STRING);")->isSuccess()); + // Enough string data to span several 4KiB WAL pages, so the torn record's reads cross a + // page boundary into a short final page. + { + // Destroy the result before tearing down the DB below. + auto insertRes = conn->query("UNWIND range(0, 7) AS i CREATE (:Blob {id: i, s: " + "repeat(CAST(i AS STRING) + '-', 1500)});"); + ASSERT_TRUE(insertRes->isSuccess()) << insertRes->getErrorMessage(); + } + auto walFilePath = lbug::storage::StorageUtils::getWALFilePath(databasePath); + ASSERT_TRUE(std::filesystem::exists(walFilePath)); + const auto walSize = std::filesystem::file_size(walFilePath); + ASSERT_GT(walSize, 8192); + conn.reset(); + database.reset(); + // Cut into the last transaction's records so the WAL ends mid-string. + std::filesystem::resize_file(walFilePath, walSize - 200); + // Strict recovery throws instead of crashing the process. + systemConfig->throwOnWalReplayFailure = true; + EXPECT_THROW(createDBAndConn(), lbug::common::Exception); + // Non-strict recovery drops the torn transaction instead of crashing. + systemConfig->throwOnWalReplayFailure = false; + createDBAndConn(); + auto res = conn->query("MATCH (b:Blob) RETURN count(*);"); + ASSERT_TRUE(res->isSuccess()) << res->getErrorMessage(); + ASSERT_TRUE(res->hasNext()); + EXPECT_EQ(res->getNext()->getValue(0)->getValue(), 0); +} + TEST_F(WalTest, WALFileLeftoverFromPreviousDBNewReadOnlyDB) { if (inMemMode || systemConfig->checkpointThreshold == 0) { GTEST_SKIP();