New issue
Advanced search Search tips

Issue 842396 link

Starred by 2 users
Status: Fixed
Owner:
alph@chromium.org
Closed: May 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 3
Type: Bug-Regression



Sign in to add a comment

Sampling heap profiler deadlocks in in access to TLS.

Project Member Reported by alph@chromium.org, May 12 2018 
Consider the stack:

    frame #2: 0x00007fff5340d4c8 libsystem_pthread.dylib`_pthread_mutex_lock_slow + 253
    frame #3: 0x0000000109aa06a8 Chromium Framework`base::internal::LockImpl::Lock(this=0x000000010f0f2080) at lock_impl_posix.cc:75 [opt]
    frame #4: 0x0000000109a39446 Chromium Framework`base::SamplingHeapProfiler::DoRecordAlloc(unsigned long, unsigned long, void*, unsigned int) [inlined] base::Lock::Acquire(this=0x000000010f0f2080) at lock.h:26 [opt]
    frame #5: 0x0000000109a39441 Chromium Framework`base::SamplingHeapProfiler::DoRecordAlloc(unsigned long, unsigned long, void*, unsigned int) [inlined] base::AutoLock::AutoLock(lock=0x000000010f0f2080) at lock.h:115 [opt]
    frame #6: 0x0000000109a39441 Chromium Framework`base::SamplingHeapProfiler::DoRecordAlloc(unsigned long, unsigned long, void*, unsigned int) [inlined] base::AutoLock::AutoLock(lock=0x000000010f0f2080) at lock.h:114 [opt]
    frame #7: 0x0000000109a39441 Chromium Framework`base::SamplingHeapProfiler::DoRecordAlloc(this=0x000000010f0f2078, total_allocated=131072, size=4096, address=0x00007fddaf89e000, skip_frames=2) at sampling_heap_profiler.cc:332 [opt]
    frame #8: 0x0000000109a3a3e3 Chromium Framework`base::(anonymous namespace)::AllocFn(self=<unavailable>, size=4096, context=<unavailable>) at sampling_heap_profiler.cc:61 [opt]
    frame #9: 0x0000000109aa1bfd Chromium Framework`base::allocator::MallocZoneFunctionsToReplaceDefault()::$_1::__invoke(_malloc_zone_t*, unsigned long) [inlined] ShimMalloc(size=4096, context=0x0000000101146000) at allocator_shim.cc:177 [opt]
    frame #10: 0x0000000109aa1be0 Chromium Framework`base::allocator::MallocZoneFunctionsToReplaceDefault()::$_1::__invoke(_malloc_zone_t*, unsigned long) [inlined] base::allocator::MallocZoneFunctionsToReplaceDefault(zone=0x0000000101146000, size=4096)::$_1::operator()(_malloc_zone_t*, unsigned long) const at allocator_shim_override_mac_symbols.h:23 [opt]
    frame #11: 0x0000000109aa1be0 Chromium Framework`base::allocator::MallocZoneFunctionsToReplaceDefault(zone=0x0000000101146000, size=4096)::$_1::__invoke(_malloc_zone_t*, unsigned long) at allocator_shim_override_mac_symbols.h:22 [opt]
    frame #12: 0x0000000109a3a3d0 Chromium Framework`base::(anonymous namespace)::AllocFn(self=<unavailable>, size=4096, context=<unavailable>) at sampling_heap_profiler.cc:60 [opt]
    frame #13: 0x0000000109aa1bfd Chromium Framework`base::allocator::MallocZoneFunctionsToReplaceDefault()::$_1::__invoke(_malloc_zone_t*, unsigned long) [inlined] ShimMalloc(size=4096, context=0x00007fff8b717000) at allocator_shim.cc:177 [opt]
    frame #14: 0x0000000109aa1be0 Chromium Framework`base::allocator::MallocZoneFunctionsToReplaceDefault()::$_1::__invoke(_malloc_zone_t*, unsigned long) [inlined] base::allocator::MallocZoneFunctionsToReplaceDefault(zone=0x00007fff8b717000, size=4096)::$_1::operator()(_malloc_zone_t*, unsigned long) const at allocator_shim_override_mac_symbols.h:23 [opt]
    frame #15: 0x0000000109aa1be0 Chromium Framework`base::allocator::MallocZoneFunctionsToReplaceDefault(zone=0x00007fff8b717000, size=4096)::$_1::__invoke(_malloc_zone_t*, unsigned long) at allocator_shim_override_mac_symbols.h:22 [opt]
    frame #16: 0x00007fff532a01bd libsystem_malloc.dylib`malloc_zone_malloc + 103
    frame #17: 0x00007fff5329f4c7 libsystem_malloc.dylib`malloc + 24
    frame #18: 0x00007fff510a7628 libc++abi.dylib`operator new(unsigned long) + 40
    frame #19: 0x0000000109a69ff1 Chromium Framework`(anonymous namespace)::ConstructTlsVector() at thread_local_storage.cc:197 [opt]
    frame #20: 0x0000000109a6a155 Chromium Framework`base::ThreadLocalStorage::Slot::Set(this=0x000000010f0f2078, value=0x000000010f0f2078) at thread_local_storage.cc:382 [opt]
    frame #21: 0x0000000109a39fbc Chromium Framework`base::SamplingHeapProfiler::DoRecordFree(void*) [inlined] base::ThreadLocalPointer<void>::Set(this=<unavailable>, ptr=0x000000010f0f2078) at thread_local.h:69 [opt]
Project Member

Comment 1 by bugdroid1@chromium.org, May 12 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0bd4e5d3ea0f9c268f1685c061843c4e924c7504

commit 0bd4e5d3ea0f9c268f1685c061843c4e924c7504
Author: Alexei Filippov <alph@chromium.org>
Date: Sat May 12 06:49:28 2018

[sampling heap profiler] Fix a deadlock.

Profiler functions used to lock the mutex first and then update the entered_
TLS variable. That caused the following sequence leading to a deadlock:

1. In DoRecordAlloc it locks the mutex and then calls entered_.Set(true)
2. If this happens to be the very first access to TLS, it causes internal
   TLS vector to be allocated.
3. It then reenters DoRecordAlloc, the entered_.Get() is still false since #1 is
   not yet finished.
4. Thus it proceed to locking the mutex again.

The fix is basically swapping the order of mutex locking and entered_.Set(true).

BUG= 842396 

Change-Id: I926c05755321c082215ca84bfc312348c89ecdca
Reviewed-on: https://chromium-review.googlesource.com/1056412
Commit-Queue: Alexei Filippov <alph@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558122}
[modify] https://crrev.com/0bd4e5d3ea0f9c268f1685c061843c4e924c7504/base/sampling_heap_profiler/sampling_heap_profiler.cc

Comment 2 by alph@chromium.org, May 14 2018

Labels: -Type-Bug Merge-Request-67 M-67 OS-Linux OS-Mac OS-Windows Type-Bug-Regression
Status: Fixed (was: Assigned)
Project Member

Comment 3 by sheriffbot@chromium.org, May 14 2018

Labels: -Merge-Request-67 Merge-Reject-67 Hotlist-Merge-Reject
The bug is marked as P3 or Feature. It should not be merged as M67 is in beta. 
Please contact the approriate milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
►

Sign in to add a comment