From 9539ee5fd3d8d0cc9c998d2ec5c15183d027c1e9 Mon Sep 17 00:00:00 2001 From: Chris Kennelly CA Date: Tue, 8 Sep 2026 16:56:21 -0700 Subject: [PATCH] Avoid data race on sampled stack trace during sampled deallocation. Unregistering the sampled allocation returns it to the free pool, allowing concurrent threads to recycle it and clobber the sampled stack trace while delete hooks are actively inspecting sampled_alloc. Copy the stack trace to a local buffer before unregistering. PiperOrigin-RevId: 978187806 --- tcmalloc/allocation_sampling.h | 12 ++++++++---- tcmalloc/huge_page_tracker.h | 26 ++++++++++++++++++-------- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/tcmalloc/allocation_sampling.h b/tcmalloc/allocation_sampling.h index ac25a46ee..58516adc7 100644 --- a/tcmalloc/allocation_sampling.h +++ b/tcmalloc/allocation_sampling.h @@ -398,6 +398,11 @@ void MaybeUnsampleAllocation(Static& state, Policy policy, static_cast(weight) / (requested_size + 1); AllocHandle sampled_alloc_handle = sampled_allocation->sampled_stack.sampled_alloc_handle; + void* stack_copy[kMaxStackDepth]; + const size_t depth = sampled_allocation->sampled_stack.depth; + TC_ASSERT_LE(depth, kMaxStackDepth); + memcpy(stack_copy, sampled_allocation->sampled_stack.stack, + depth * sizeof(void*)); MallocHook::SampledAlloc sampled_alloc = { .handle = sampled_alloc_handle, .requested_size = requested_size, @@ -405,8 +410,7 @@ void MaybeUnsampleAllocation(Static& state, Policy policy, sampled_allocation->sampled_stack.requested_alignment, .allocated_size = allocated_size, .weight = allocation_estimate, - .stack = absl::MakeSpan(sampled_allocation->sampled_stack.stack, - sampled_allocation->sampled_stack.depth), + .stack = absl::MakeSpan(stack_copy, depth), .allocation_time = sampled_allocation->sampled_stack.allocation_time, .ptr = ptr, .access_hint = sampled_allocation->sampled_stack.access_hint, @@ -414,7 +418,6 @@ void MaybeUnsampleAllocation(Static& state, Policy policy, ? MallocHook::Access::Cold : MallocHook::Access::Hot, }; - state.sampled_allocation_recorder().Unregister(sampled_allocation); // Adjust our estimate of internal fragmentation. TC_ASSERT_LE(requested_size, allocated_size); @@ -427,8 +430,9 @@ void MaybeUnsampleAllocation(Static& state, Policy policy, sampled_fragmentation); state.sampled_internal_fragmentation_.Add(-sampled_fragmentation); } - MallocHook::InvokeSampledDeleteHook(sampled_alloc); + state.sampled_allocation_recorder().Unregister(sampled_allocation); + MallocHook::InvokeSampledDeleteHook(sampled_alloc); state.deallocation_samples.ReportFree(sampled_alloc_handle); } diff --git a/tcmalloc/huge_page_tracker.h b/tcmalloc/huge_page_tracker.h index fa9628411..be23a9e96 100644 --- a/tcmalloc/huge_page_tracker.h +++ b/tcmalloc/huge_page_tracker.h @@ -437,8 +437,7 @@ inline PageTracker::HardwarePageResidencyInfo PageTracker::CountInfoInHugePage( } TC_ASSERT_LE(kHardwarePagesInHugePage, kMaxResidencyBits); - const PageBitmap& used = free_.bits(); - const PageBitmap free = ~used; + const PageBitmap& free = free_.bits(); TC_ASSERT_EQ(kHardwarePagesInHugePage % kPagesPerHugePage.raw_num(), 0); const int shift = kHardwarePagesInHugePage / kPagesPerHugePage.raw_num(); @@ -446,12 +445,23 @@ inline PageTracker::HardwarePageResidencyInfo PageTracker::CountInfoInHugePage( TC_ASSERT_LT((kHardwarePagesInHugePage - 1) >> shift_bits, kPagesPerHugePage.raw_num()); - return {.n_free_swapped = (free & swapped).CountBits() * shift, - .n_used_swapped = (used & swapped).CountBits() * shift, - .n_free_unbacked = (free & unbacked).CountBits() * shift, - .n_used_unbacked = (used & unbacked).CountBits() * shift, - .n_free_stale = (free & stale).CountBits() * shift, - .n_used_stale = (used & stale).CountBits() * shift}; + size_t n_unbacked[2] = {0, 0}; + size_t n_swapped[2] = {0, 0}; + size_t n_stale[2] = {0, 0}; + + n_unbacked[0] = (free & unbacked).CountBits() * shift; + n_unbacked[1] = (~free & unbacked).CountBits() * shift; + n_swapped[0] = (free & swapped).CountBits() * shift; + n_swapped[1] = (~free & swapped).CountBits() * shift; + n_stale[0] = (free & stale).CountBits() * shift; + n_stale[1] = (~free & stale).CountBits() * shift; + + return {.n_free_swapped = n_swapped[1], + .n_used_swapped = n_swapped[0], + .n_free_unbacked = n_unbacked[1], + .n_used_unbacked = n_unbacked[0], + .n_free_stale = n_stale[1], + .n_used_stale = n_stale[0]}; } inline void PageTracker::Put(Range r, SpanAllocInfo span_alloc_info) {