New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 759138 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Sep 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Stack overflow in --memlog on shutdown on macOS

Project Member Reported by erikc...@chromium.org, Aug 25 2017

Issue description

"""
    frame #60248: 0x0000000119cb1a90 libbase.dylib`::__invoke() + 32 at allocator_shim_override_mac_symbols.h:22
    frame #60249: 0x00007fffc5832282 libsystem_malloc.dylib`malloc_zone_malloc + 107
    frame #60250: 0x00007fffc5831200 libsystem_malloc.dylib`malloc + 24
    frame #60251: 0x00007fffc42b9e0e libc++abi.dylib`operator new(unsigned long) + 30
    frame #60252: 0x00007fffc427e62c libc++.1.dylib`std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::__grow_by(unsigned long, unsigned long, unsigned long, unsigned long, unsigned long, unsigned long) + 138
    frame #60253: 0x00007fffc427e30e libc++.1.dylib`std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::push_back(char) + 86
    frame #60254: 0x000000011980aa1c libbase.dylib`::overflow() + 364 at sstream:535
    frame #60255: 0x00007fffc424ac5a libc++.1.dylib`std::__1::basic_streambuf<char, std::__1::char_traits<char> >::xsputn(char const*, long) + 112
    frame #60256: 0x00000001198063f0 libbase.dylib`::__pad_and_output<char, std::__1::char_traits<char> >() [inlined] sputn + 1104 at streambuf:227
    frame #60257: 0x00000001198063d6 libbase.dylib`::__pad_and_output<char, std::__1::char_traits<char> >() + 1078 at locale:1414
    frame #60258: 0x0000000119805e4e libbase.dylib`::__put_character_sequence<char, std::__1::char_traits<char> >() + 750 at ostream:725
    frame #60259: 0x00000001198f6e94 libbase.dylib`::operator<<<std::__1::char_traits<char> >() + 52 at ostream:790
    frame #60260: 0x000000011990fdce libbase.dylib`::Init() + 1646 at logging.cc:825
    frame #60261: 0x000000011990f729 libbase.dylib`::LogMessage() + 969 at logging.cc:530
    frame #60262: 0x0000000119910139 libbase.dylib`::LogMessage() + 41 at logging.cc:529
    frame #60263: 0x000000012fa426f6 libmojo_system_impl.dylib`::PlatformChannelWrite() + 134 at platform_channel_utils_posix.cc:112
    frame #60264: 0x000000010532e1df libchrome_dll.dylib`::Send() + 79 at memlog_sender_pipe_posix.cc:28
    frame #60265: 0x000000010532a64b libchrome_dll.dylib`::SendCurrentBuffer() + 43 at memlog_allocator_shim.cc:64
    frame #60266: 0x000000010532a5bb libchrome_dll.dylib`::Send() + 91 at memlog_allocator_shim.cc:56
    frame #60267: 0x000000010532a05d libchrome_dll.dylib`::DoSend() + 109 at memlog_allocator_shim.cc:83
    frame #60268: 0x0000000105329fc4 libchrome_dll.dylib`::AllocatorShimLogAlloc() + 356 at memlog_allocator_shim.cc:232
    frame #60269: 0x000000010532a1d1 libchrome_dll.dylib`::HookAlloc() + 81 at memlog_allocator_shim.cc:90
    frame #60270: 0x0000000119cb1afe libbase.dylib`::ShimMalloc() + 46 at allocator_shim.cc:194
    frame #60271: 0x0000000119cb1ac4 libbase.dylib`::operator()() + 36 at allocator_shim_override_mac_symbols.h:23
    frame #60272: 0x0000000119cb1a90 libbase.dylib`::__invoke() + 32 at allocator_shim_override_mac_symbols.h:22
"""

Line in question:
ssize_t PlatformChannelWrite(PlatformHandle h,
                             const void* bytes,
                             size_t num_bytes) {
>>>  DCHECK(h.is_valid());

Interesting. DCHECKs cause recursion [unsurprisingly]. So there's actually two problems.

1) During shutdown, we're closing the pipe to the profiling service but still recording allocations.
2) We must not do any logging in the critical path for recording allocations. Or we need a mechanism to disable re-entrancy. 
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 31 2017

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

commit 46c23513c3729389a320f560410068da8127ac2f
Author: Erik Chen <erikchen@chromium.org>
Date: Thu Aug 31 21:22:36 2017

Fix shutdown of out-of-process heap-profiling.

Previously, during shutdown, the destructor of MemlogClient would close its pipe
to the profiling process. This in turn would cause all future memory allocations
to hit a recursive loop, as they have a DCHECK checking to make sure that the
pipe is still valid.

This CL attempts to [as gracefully as possible], shut down the memlog portion of
the allocation shim. There is no way to do this synchronously and consistently
without harming performance of the fast path for malloc. Instead, we also leak
the pipe from MemlogClient, with the idea that there will be relatively few
allocations that will still use the pipe.

Bug:  759138 
Change-Id: I727c1162b311b4e876a31b743551df8ac48b4460
Reviewed-on: https://chromium-review.googlesource.com/646599
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499004}
[modify] https://crrev.com/46c23513c3729389a320f560410068da8127ac2f/chrome/common/profiling/memlog_allocator_shim.cc
[modify] https://crrev.com/46c23513c3729389a320f560410068da8127ac2f/chrome/common/profiling/memlog_allocator_shim.h
[modify] https://crrev.com/46c23513c3729389a320f560410068da8127ac2f/chrome/common/profiling/memlog_client.cc

Status: Fixed (was: Untriaged)

Sign in to add a comment