bcaf2794dc
Summary:
InjectedErrorLog is a lock-free circular ring buffer designed to be safe to call from signal handlers (which cannot use locks). It has an intentional benign data race between Record() (called by worker threads) and PrintAll() (called by the main thread from a signal/termination handler). The code documents this trade-off in comments, but was missing TSAN suppression annotations.
Simply adding TSAN_SUPPRESSION (__attribute__((no_sanitize("thread")))) is insufficient because TSAN still intercepts libc functions like vsnprintf/snprintf -- accesses through these interceptors are still tracked even when the calling function is annotated.
The fix:
1. Add TSAN_SUPPRESSION to both Record() and PrintAll() to suppress direct field reads/writes in the function body.
2. Restructure both functions to use local stack buffers for vsnprintf/snprintf operations instead of operating directly on shared entry data. This avoids passing shared memory through TSAN-intercepted libc functions.
Also adds fault_injection_fs_test with a ConcurrentRecordAndPrintAll test that exercises the concurrent Record() + PrintAll() pattern and verifies no TSAN race is reported.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/14467
Test Plan:
- fault_injection_fs_test passes under TSAN (buck2 test fbcode//mode/dbg-tsan)
- Reverted fix, re-ran: Fatal (6 TSAN warnings) -- round-trip confirmed
- fault_injection_fs_test passes under debug mode (no regression)
Reviewed By: mszeszko-meta
Differential Revision: D96948483
Pulled By: xingbowang
fbshipit-source-id: efdd5eafa12a5a82f973e40aa327901cc5f95033
94 lines
2.8 KiB
C++
94 lines
2.8 KiB
C++
// Copyright (c) 2011-present, Facebook, Inc. All rights reserved.
|
|
// This source code is licensed under both the GPLv2 (found in the
|
|
// COPYING file in the root directory) and Apache 2.0 License
|
|
// (found in the LICENSE.Apache file in the root directory).
|
|
|
|
#include "utilities/fault_injection_fs.h"
|
|
|
|
#include <atomic>
|
|
#include <thread>
|
|
#include <vector>
|
|
|
|
#include "test_util/testharness.h"
|
|
|
|
namespace ROCKSDB_NAMESPACE {
|
|
|
|
class InjectedErrorLogTest : public testing::Test {};
|
|
|
|
// Test basic Record and PrintAll functionality.
|
|
TEST_F(InjectedErrorLogTest, BasicRecordAndPrint) {
|
|
InjectedErrorLog log;
|
|
log.SetLogFilePath("/dev/null");
|
|
|
|
// Record some entries.
|
|
log.Record("op=Get key=0x%08x status=%s", 0x12345678, "OK");
|
|
log.Record("op=Put key=0x%08x value_size=%d", 0xABCDEF00, 100);
|
|
log.Record("op=Delete key=0x%08x", 0x00000001);
|
|
|
|
// PrintAll should not crash.
|
|
log.PrintAll();
|
|
}
|
|
|
|
// Test that the circular buffer wraps correctly.
|
|
TEST_F(InjectedErrorLogTest, CircularBufferWrap) {
|
|
InjectedErrorLog log;
|
|
log.SetLogFilePath("/dev/null");
|
|
|
|
// Fill beyond kMaxEntries to trigger wraparound.
|
|
for (size_t i = 0; i < InjectedErrorLog::kMaxEntries + 100; i++) {
|
|
log.Record("entry=%zu", i);
|
|
}
|
|
|
|
// PrintAll should handle the wrapped buffer without crashing.
|
|
log.PrintAll();
|
|
}
|
|
|
|
// Test concurrent Record() from multiple threads.
|
|
// Keep total records (kNumThreads * kRecordsPerThread) under kMaxEntries
|
|
// to avoid write-write races from buffer wraparound, which are benign but
|
|
// would trigger TSAN warnings.
|
|
TEST_F(InjectedErrorLogTest, ConcurrentRecord) {
|
|
InjectedErrorLog log;
|
|
constexpr int kNumThreads = 4;
|
|
constexpr int kRecordsPerThread = 200;
|
|
static_assert(kNumThreads * kRecordsPerThread <
|
|
static_cast<int>(InjectedErrorLog::kMaxEntries),
|
|
"total records must stay within buffer to avoid TSAN-visible "
|
|
"write-write races on overlapping slots");
|
|
|
|
std::vector<std::thread> threads;
|
|
threads.reserve(kNumThreads);
|
|
for (int t = 0; t < kNumThreads; t++) {
|
|
threads.emplace_back([&log, t]() {
|
|
for (int i = 0; i < kRecordsPerThread; i++) {
|
|
log.Record("thread=%d iter=%d op=Get key=0x%08x", t, i, i * 17);
|
|
}
|
|
});
|
|
}
|
|
|
|
for (auto& t : threads) {
|
|
t.join();
|
|
}
|
|
|
|
// PrintAll after all threads are done -- no race.
|
|
log.SetLogFilePath("/dev/null");
|
|
log.PrintAll();
|
|
}
|
|
|
|
// Test HexHead utility.
|
|
TEST_F(InjectedErrorLogTest, HexHead) {
|
|
const char data[] = "\x01\x02\xAB\xCD";
|
|
std::string result = InjectedErrorLog::HexHead(data, 4);
|
|
ASSERT_EQ(result, "01 02 ab cd");
|
|
|
|
result = InjectedErrorLog::HexHead(data, 4, 2);
|
|
ASSERT_EQ(result, "01 02 ...");
|
|
}
|
|
|
|
} // namespace ROCKSDB_NAMESPACE
|
|
|
|
int main(int argc, char** argv) {
|
|
ROCKSDB_NAMESPACE::port::InstallStackTraceHandler();
|
|
::testing::InitGoogleTest(&argc, argv);
|
|
return RUN_ALL_TESTS();
|
|
}
|