advisedy opened a new pull request, #3559: URL: https://github.com/apache/kvrocks/pull/3559
Fixes #3100 ## Problem TSan reported an oversized allocation: ``` ThreadSanitizer: requested allocation size 0x100000000000000 exceeds maximum supported size of 0x10000000000 ``` Root cause: Before the fix, `GetFullReplDataInfo` released the lock before calling `GetChildren`, while `Server::cron` called `rocksdb::DestroyDB` on the live checkpoint directory without holding the lock at all. This allowed the following interleaving: ``` meta: unlock checkpoint_mu_ purge: DestroyDB starts deleting files meta: GetChildren → [] (dir exists but files are already gone) ``` `GetFullReplDataInfo` builds the file list by appending entries and then calls `files->pop_back()`. If the directory is empty, `files` is an empty string, and `pop_back` on an empty string is undefined behavior. (https://en.cppreference.com/cpp/string/basic_string/pop_back) On libstdc++, the common manifestation is a size underflow that produces a very large number. The subsequent `files + CRLF` then allocates memory based on this corrupted size, triggering the TSan error. eaxmple: ```cpp #include <cstdio> #include <string> int main() { std::string files; files.pop_back(); auto s = files + "\r\n"; printf("size=%zu\n", files.size()); return 0; } ``` Running with GCC 11 + ASan on Linux: ``` AddressSanitizer: requested allocation size 0x100000000000000 (0x100000000001000 after adjustments for alignment, red zones etc.) exceeds maximum supported size of 0x10000000000 (thread T0) #0 0x7ffffee8b1e7 in operator new(unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:99 #1 0x7ffffecf8109 in void std::__cxx11::basic_string::_M_construct<char*>(...) (/lib/x86_64-linux-gnu/libstdc++.so.6+0x14f109) ... SUMMARY: AddressSanitizer: allocation-size-too-big ``` The corrupted size (`0x100000000000000`) and the threshold (`0x10000000000`) match the TSan report in the issue. While investigating, we also found that the same race condition, besides the "empty list" extreme case, could cause the replica to receive an incomplete file list (`GetChildren` returned only a subset of files). --- ## Fix No `pop_back` on empty list： In `GetFullReplDataInfo`, if `files` is empty, return `NotOK` immediately instead of calling `pop_back`. Introduce `TryPurgeCheckpoint` — rename then destroy： Added `Storage::TryPurgeCheckpoint`, which changes the purge logic to: 1. Under `checkpoint_mu_`, atomically rename `checkpoint_dir` to `checkpoint_dir.trash` via `RenameFile`; 2. Clear `checkpoint_info_`; 3. After releasing the lock, call `DestroyDB(trash)`. This way, the meta listing either sees a complete live directory or no directory at all. --- ## Tests Two new C++ gtests added (`storage_test.cc`): - `GetFullReplDataInfoRejectsEmptyCheckpoint`: Creates a checkpoint → clears the directory → asserts `NotOK`. - `TryPurgeCheckpointPolicyAndAtomicRemove`: Verifies idle purge, rename atomicity, and `checkpoint_info_` reset. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
