Skip to content

Commit 0a49671

Browse files
jbachorikclaude
andcommitted
fix(tsan): use atomic reads in findCallTrace to eliminate data race
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent b3f7d38 commit 0a49671

2 files changed

Lines changed: 75 additions & 3 deletions

File tree

ddprof-lib/src/main/cpp/callTraceHashTable.cpp

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -232,10 +232,19 @@ CallTrace *CallTraceHashTable::findCallTrace(LongHashTable *table, u64 hash) {
232232

233233
u32 slot = probe.slot();
234234
while (true) {
235-
if (keys[slot] == hash) {
236-
return table->values()[slot].trace;
235+
// Use atomic load: keys[] can be written concurrently via CAS in put()
236+
// when a table is promoted to prev but still has in-flight insertions.
237+
u64 key = __atomic_load_n(&keys[slot], __ATOMIC_ACQUIRE);
238+
if (key == hash) {
239+
// Use acquireTrace() to pair with the RELEASE store in setTrace().
240+
// If still PREPARING, treat as not found: callers will create a new entry.
241+
CallTrace *trace = table->values()[slot].acquireTrace();
242+
if (trace == CallTraceSample::PREPARING) {
243+
return nullptr;
244+
}
245+
return trace;
237246
}
238-
if (keys[slot] == 0) {
247+
if (key == 0) {
239248
return nullptr;
240249
}
241250
if (!probe.hasNext()) {

ddprof-lib/src/test/cpp/stress_callTraceStorage.cpp

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2014,6 +2014,69 @@ TEST_F(StressTestSuite, HashTableSpinWaitEdgeCasesTest) {
20142014
EXPECT_LT(drop_rate, 0.5) << "Excessive trace drop rate: " << drop_rate;
20152015
}
20162016

2017+
// Regression test: TSan data race in findCallTrace() — keys[] plain reads
2018+
// raced with atomic CAS writes from concurrent put(). When a table is
2019+
// promoted to prev after expansion, threads that loaded the old table
2020+
// pointer before the CAS swap still write into it while new threads call
2021+
// findCallTrace(prev, hash) — the read must be atomic.
2022+
TEST_F(StressTestSuite, FindCallTraceAtomicReadRaceTest) {
2023+
// Fill the table to just below the 75% expansion threshold
2024+
// (INITIAL_CAPACITY = 65536; 75% = 49152).
2025+
const int FILL_TARGET = 48800;
2026+
const int NUM_THREADS = 16;
2027+
const int OPS_PER_THREAD = 400;
2028+
2029+
void* aligned_memory = std::aligned_alloc(alignof(CallTraceHashTable), sizeof(CallTraceHashTable));
2030+
ASSERT_NE(aligned_memory, nullptr);
2031+
auto hash_table_ptr = std::unique_ptr<CallTraceHashTable, void(*)(CallTraceHashTable*)>(
2032+
new(aligned_memory) CallTraceHashTable(),
2033+
[](CallTraceHashTable* ptr) {
2034+
ptr->~CallTraceHashTable();
2035+
std::free(ptr);
2036+
}
2037+
);
2038+
CallTraceHashTable& hash_table = *hash_table_ptr;
2039+
hash_table.setInstanceId(42);
2040+
2041+
// Pre-fill single-threaded up to just below the expansion threshold
2042+
for (int i = 0; i < FILL_TARGET; ++i) {
2043+
ASGCT_CallFrame frame;
2044+
frame.bci = i + 1;
2045+
frame.method_id = reinterpret_cast<jmethodID>(0x10000 + i);
2046+
hash_table.put(1, &frame, false, 1);
2047+
}
2048+
2049+
// Release all threads simultaneously so some cross the 75% threshold,
2050+
// triggering expansion. Threads that loaded the old table pointer before
2051+
// the expansion CAS will continue inserting into the now-prev table while
2052+
// threads that see the new table call findCallTrace(prev, hash).
2053+
std::atomic<bool> go{false};
2054+
std::atomic<uint64_t> successes{0};
2055+
std::vector<std::thread> workers;
2056+
2057+
for (int t = 0; t < NUM_THREADS; ++t) {
2058+
workers.emplace_back([&, t]() {
2059+
while (!go.load(std::memory_order_acquire)) { /* spin */ }
2060+
for (int i = 0; i < OPS_PER_THREAD; ++i) {
2061+
ASGCT_CallFrame frame;
2062+
frame.bci = FILL_TARGET + 1 + t * OPS_PER_THREAD + i;
2063+
frame.method_id = reinterpret_cast<jmethodID>(0x80000 + t * OPS_PER_THREAD + i);
2064+
u64 id = hash_table.put(1, &frame, false, 1);
2065+
if (id != 0 && id != CallTraceStorage::DROPPED_TRACE_ID) {
2066+
successes.fetch_add(1, std::memory_order_relaxed);
2067+
}
2068+
}
2069+
});
2070+
}
2071+
2072+
go.store(true, std::memory_order_release);
2073+
for (auto& w : workers) {
2074+
w.join();
2075+
}
2076+
2077+
EXPECT_GT(successes.load(), 0u) << "No successful insertions after expansion";
2078+
}
2079+
20172080
// Test 13: Hash Table Memory Allocation Failure Stress Test
20182081
TEST_F(StressTestSuite, HashTableAllocationFailureStressTest) {
20192082
const int NUM_THREADS = 8;

0 commit comments

Comments
 (0)