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

Issue 665567 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug


Sign in to add a comment

Port allocator shim to Mac OSX

Project Member Reported by erikc...@chromium.org, Nov 15 2016

Issue description

macOS: 10.11.6
Chrome: Canary or ToT

Start Chrome with --enable-heap-profiling. Take a memory-infra trace. Click on a memory icon. Note that many of the columns have a triple-hotdog view showing more information, but the malloc column does not.
 
Blocking: 550886
Summary: Port allocator shim to Mac OSX (was: "malloc" column doesn't get detailed background when --enable-heap-profiling is enabled.)
This is WAI due to the lack of the allocator shim on Mac OSX.
Issue 550886 has more context about the shim.

Design doc: https://bit.ly/allocator-shim

Essentially there has been quite some work to make sure that on all the other platforms, calls to malloc / new / delete / free and friend all go through //base/allocator/allocator_shim
This:
1) allows to make some security checks all in one place (suicide on OOM and similar)
2) allows things like memory-infra to hook malloc/free calls and snoop allocations to do its bookkeping.

Today the shim is not there on MacOSX and this is the reason why you see this behavior.

Concretely speaking it shouldn't be too hard to shim OSX.
OSX already intercepts malloc()/free() and friends but it does in its own place (base/process/memory_mac.mm).
That should be ported and match the model of //base/allocator/allocator_shim*
Once that is done memory-infra will work for free.
Cc: dskiba@chromium.org
+dskiba I think he tried something recently

Comment 3 by dskiba@chromium.org, Nov 15 2016

Here is my mail from back then:

"
Hi,

I've hacked together an allocator shim for macOS to be able to do work on my MacBook. CL is here: https://codereview.chromium.org/2499373003 It's based on base/process/memory_mac.mm. I also fixed tests to pass.

Things learned:

1. AllocatorDispatch::GetSizeEstimateFn() should take 'const void*', not 'void*'. I had to do nasty const_cast<> in ShimGetSizeEstimate(). Both HeapSize (on Windows) and malloc_usable_size() take 'const void*'.

2. macOS realloc() is not a simple stub to zone->realloc(). It looks up malloc zone for 'ptr' argument (and aborts if that fails, so trick with realloc(0x420) from tests doesn't work.). And if 'ptr' argument is nullptr, it simply calls zone->malloc(). I had to fix / disable tests.

3. Some functions in malloc zone depend on version. For example for version >= 6 zone->free_definite_size() is called instead of zone->free(). So I had to hook it too.

4. attribute((alias("fn"))) is not supported on macOS. attribute((weakref("fn"))) is supported, but only if the function has internal linkage. So I had to reimplement new/delete operators and just call appropriate ShimCppXXX functions.

5. There is a difference between ShimCppNew and ShimMalloc, but it's not tested by InterceptCppSymbols test.

6. Also updated AllocatorShimTest to hook get_size_estimation.

TODO:

1. base/process/memory_mac.mm hooks more stuff - purgeable_zone, CFAllocators and even [NSObject allocWithZone:]. Allocator shim probably should too.

2. Whole process of hooking memory zones must be triggered at runtime, it can't happen statically. Right now this is done in a ctor of a global object. Maybe there is better way (__attribute__((constructor(0)))?).

3. Fix InterceptCppSymbols test.

4. Fix AllocatorDispatch::GetSizeEstimateFn() to take 'const void*' (might do this myself).
"

Comment 4 by l...@chromium.org, Nov 16 2016

Owner: alph@chromium.org
@alph, might this be related to: 665536 ?
Cc: alph@chromium.org
Components: -Platform>DevTools>Memory Internals>Tracing
Labels: Hotlist-MemoryInfra
Owner: ----
Status: Available (was: Untriaged)
This is not a devtools issue. I know that there is a proiler also there, but this different and is relative to https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/heap_profiler.md

Issue 665536 is orthogonal, the reason is simply code missing as explained in #1.
Happy if somebody wants to champion this area but let's not overload our devtools friends with bugs that are not their :)
Labels: Hotlist-GoodFirstBug
Labels: -Hotlist-GoodFirstBug
Owner: kraynov@chromium.org
Status: Assigned (was: Available)
I did some experiments, will implement this feature.
Awesome! Thank you :)
Owner: erikc...@chromium.org
It looks like we have a brave volunteer to take this over.
Thanks Erik!
There are several potential mechanisms for overriding malloc. tcmalloc's documentation describes why the approach [malloc zone modification] currently used in Chrome, and in dskiba's CL is better than the alternatives:
https://cs.chromium.org/chromium/src/third_party/tcmalloc/chromium/src/libc_override_osx.h?l=37

The main downside is that this misses out on allocations before malloc zone modification. tcmalloc uses a different mechanism for modifying the malloc zone. https://cs.chromium.org/chromium/src/third_party/tcmalloc/chromium/src/libc_override_osx.h?l=271

This has the benefit of correctly handling allocations made before the zone change. Also, it does not require calling mach_vm_region/mach_vm_protect. 
As an aside, jemalloc does the same thing as tcmalloc:
https://github.com/jemalloc/jemalloc/blob/dev/src/zone.c

But also registers/unregisters the purgeable zone.
Reading the source for jemalloc shows that 10.12 also has a "lite zone" when malloc stack logging is enabled. Upstream tcmalloc has the same code/comment:
https://github.com/gperftools/gperftools/blob/master/src/libc_override_osx.h
Re #10: my main concern there is stability. These allocator change can have very subtle effects that are very hard to predict, see historical comments in [1].
If I was doing this, I'd very paranoidly just  adapt what we have today which is guaranteed to work.
At the same time, I don't know absolutely anything about OSX, you folks are the experts here so I won't rule out (nor I'm going to oppose to) other, better, approaches. Just my tip here would be make sure that OSX base owners (mark@ and rsesek@ might be good candidates) are fine with that.

Also note that we never used tcmalloc (the one in third_party) on OSX, we always used the system one. The only cases in which we use(d) the checked in version of tcmalloc are:
- Linux: since forever and still today.
- Android: only in custom profiling builds for a while to make DMP (Deep Memory Profiler, now nuked from orbit) work. Was quite crashy.
- Windows: there was a time where tcmalloc was used there. But then we switched to WinHeap a while ago.
- OSX: never

So all the OSX code that you see there has never been hit in chrome and, as far as we know, might not work.

[1] https://cs.chromium.org/chromium/src/third_party/tcmalloc/vendor/src/libc_override_glibc.h?rcl=0&l=66
Cc: mark@chromium.org
+mark [macOS guru]

All heap-based memory allocations will eventually go through the mach vm subsystem. On macOS, malloc [libsystem_malloc.dylib, bsd/kern/kern_malloc.c] uses an additional abstraction layer of zones. Most frameworks will use the default zone [or perhaps the default purgeable zone], but frameworks are also allowed to register their own zones [and presumably use a custom implementation based directly on the mach vm system calls]. 

When I launch Chrome through lldb and log calls to malloc_zone_register and malloc_create_zone in the browser process, I see three new zones registered from libGLImage [malloc_create_zone], QuartzCore [malloc_zone_register] and ImageIO [malloc_zone_register]. When I print information about all zones in the process, I see 5:
  * default zone [~32MB]
  * purgeable zone, libGLImage, QuartzCore, ImageIO [no allocations]
Presumably the default zone and purgeable zone were inherited from the parent process.

My examination of the implementation of CFAllocator [CoreFoundation, see _CFAllocatorAllocate] and NSZone [libobjc, see _class_createInstanceFromZone] show that they call through to malloc zones. 

Overriding all malloc zones will allow us to intercept non-shared-memory heap allocations. There is no way [short of interposition] of determining when new zones are registered. Luckily, most memory appears to be allocated through the default zone. Given that malloc zones expose their total memory usage, I suggest that we only intercept default malloc zone allocations for the allocator shim. We will still know how much memory we're failing to track in the allocator shim.
In the future, if we deem it necessary, we could theoretically track zones independently [and even have pseudo-zones for CoreFoundation and ObjC objects]. The main problem is that we would have to refactor the current implementation of the allocator shim [https://cs.chromium.org/chromium/src/base/allocator/README.md]. This seems like premature optimization to me - we can do it we think it's necessary.

In the short term, we should:
1) Get the patch from c#3 working. 
  1a) We need to make sure batch_malloc and batch_free are intercepted as they get used by CF and ObjC, which the patch does not do.
  1b) Fix the other issues raised in c#3.

In the medium term, we should:
2) Figure out the right way to intercept malloc zone calls. Note that jemalloc and tcmalloc [see c#10, c#11] accomplish this without needing to change the protection on pages. This seems cleaner. mark@, perhaps you can comment?
whoops, and the source link for malloc should be [libsystem_malloc.dylib, libmalloc-116/src/malloc.c]

Comment 17 by mark@chromium.org, Jan 3 2017

Responding #c15: I don’t know that what jemalloc and tcmalloc do is any “cleaner.” Certainly as far as the custom allocator zone itself is concerned, it’s cleaner, but the “promotion” of the custom zone to “default” is tricky and fragile. That’s not to say that the alternatives aren’t tricky or fragile, and certainly we’d want test coverage to ensure that we retain our desired behavior over time (and with OS updates) in any case. I’m just pointing out that each approach has its quirks.

Anyway, I’m not sure that I’d call out the protection stuff as a limitation per se. When you register your own zone, libmalloc’s going to need to twiddle the protection bits momentarily too. Sure, there’s a difference in that it handles it for you, and it handles it under its own lock so there’s less room for racy badness, so that’s nice. I’m only mentioning it because you mentioned protection a couple of times, but that doesn’t really stand out as a prime concern to me. Like I said, the nicest thing about what jemalloc and tcmalloc do is that they use the zone feature in a more natural way, so that service for preexisting allocations doesn’t wind up going to a zone that’s never heard of the memory.

On the other hand, with a good enough way to route preexisting allocations to their original zone, it’s awfully nice in some cases to be able to take over allocation requests to non-default zones too…
Components: Internals>Instrumentation>Memory
Components: -Internals>Tracing
Blocking: 680194
Project Member

Comment 21 by bugdroid1@chromium.org, Jan 26 2017

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

commit 9550b71347ce9acbe2875fb75ea46ec096230506
Author: erikchen <erikchen@chromium.org>
Date: Thu Jan 26 05:36:58 2017

Move logic from memory_mac to allocator_interception_mac.

This CL is a refactor and has no intended behavior changes.

This CL moves the allocator interception logic from base/process/memory_mac.mm
to base/allocator/allocator_interception_mac.mm in preparation for an
implementation of the allocator shim.

BUG= 665567 

Review-Url: https://codereview.chromium.org/2649973003
Cr-Commit-Position: refs/heads/master@{#446234}

[modify] https://crrev.com/9550b71347ce9acbe2875fb75ea46ec096230506/base/BUILD.gn
[add] https://crrev.com/9550b71347ce9acbe2875fb75ea46ec096230506/base/allocator/allocator_interception_mac.h
[add] https://crrev.com/9550b71347ce9acbe2875fb75ea46ec096230506/base/allocator/allocator_interception_mac.mm
[modify] https://crrev.com/9550b71347ce9acbe2875fb75ea46ec096230506/base/process/memory_mac.mm

Project Member

Comment 22 by bugdroid1@chromium.org, Jan 26 2017

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

commit 50a11072abe7c112ed23956929b3ac4d0590f6ff
Author: erikchen <erikchen@chromium.org>
Date: Thu Jan 26 06:35:43 2017

Refactor allocator_interception_mac to use {Store,Replace}ZoneFunctions.

This CL has no intended behavior change.

Pulling out the logic for {Store,Replace}ZoneFunctions allows the allocator shim
to easily integrate with the existing malloc interception code.

BUG= 665567 

Review-Url: https://codereview.chromium.org/2650363002
Cr-Commit-Position: refs/heads/master@{#446255}

[modify] https://crrev.com/50a11072abe7c112ed23956929b3ac4d0590f6ff/base/allocator/allocator_interception_mac.h
[modify] https://crrev.com/50a11072abe7c112ed23956929b3ac4d0590f6ff/base/allocator/allocator_interception_mac.mm

Project Member

Comment 23 by bugdroid1@chromium.org, Jan 26 2017

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

commit e9942e23fec9e2bca8ebd70f79feb2120943b54b
Author: ajuma <ajuma@chromium.org>
Date: Thu Jan 26 16:35:31 2017

Revert of Move logic from memory_mac to allocator_interception_mac. (patchset #5 id:80001 of https://codereview.chromium.org/2649973003/ )

Reason for revert:
This seems to be causing compile to fail on the Mac GPU FYI Asan bot, with error:
../../base/allocator/allocator_interception_mac.mm:320:3: error: use of undeclared identifier 'DeprotectMallocZone'
  DeprotectMallocZone(zone, &reprotection_start, &reprotection_length,
  ^
1 error generated.

See https://build.chromium.org/p/chromium.gpu.fyi/builders/Mac%20GPU%20ASAN%20Release/builds/1407/steps/compile/logs/stdio
and
https://build.chromium.org/p/chromium.gpu.fyi/builders/Mac%20GPU%20ASAN%20Release?numbuilds=100

Original issue's description:
> Move logic from memory_mac to allocator_interception_mac.
>
> This CL is a refactor and has no intended behavior changes.
>
> This CL moves the allocator interception logic from base/process/memory_mac.mm
> to base/allocator/allocator_interception_mac.mm in preparation for an
> implementation of the allocator shim.
>
> BUG= 665567 
>
> Review-Url: https://codereview.chromium.org/2649973003
> Cr-Commit-Position: refs/heads/master@{#446234}
> Committed: https://chromium.googlesource.com/chromium/src/+/9550b71347ce9acbe2875fb75ea46ec096230506

TBR=primiano@chromium.org,mark@chromium.org,erikchen@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 665567 

Review-Url: https://codereview.chromium.org/2658083002
Cr-Commit-Position: refs/heads/master@{#446346}

[modify] https://crrev.com/e9942e23fec9e2bca8ebd70f79feb2120943b54b/base/BUILD.gn
[delete] https://crrev.com/cb3a30a6acb4e3e774f9c0d416bb90c270065364/base/allocator/allocator_interception_mac.h
[delete] https://crrev.com/cb3a30a6acb4e3e774f9c0d416bb90c270065364/base/allocator/allocator_interception_mac.mm
[modify] https://crrev.com/e9942e23fec9e2bca8ebd70f79feb2120943b54b/base/process/memory_mac.mm

Project Member

Comment 24 by bugdroid1@chromium.org, Jan 26 2017

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

commit 9dbabee0d6c79ecf51ba5cc5edc7cd7d02f6b4fa
Author: erikchen <erikchen@chromium.org>
Date: Thu Jan 26 19:22:32 2017

Move logic from memory_mac to allocator_interception_mac.

This CL is a refactor and has no intended behavior changes.

This CL moves the allocator interception logic from base/process/memory_mac.mm
to base/allocator/allocator_interception_mac.mm in preparation for an
implementation of the allocator shim.

BUG= 665567 

Review-Url: https://codereview.chromium.org/2649973003
Cr-Original-Commit-Position: refs/heads/master@{#446234}
Committed: https://chromium.googlesource.com/chromium/src/+/9550b71347ce9acbe2875fb75ea46ec096230506
Review-Url: https://codereview.chromium.org/2649973003
Cr-Commit-Position: refs/heads/master@{#446385}

[modify] https://crrev.com/9dbabee0d6c79ecf51ba5cc5edc7cd7d02f6b4fa/base/BUILD.gn
[add] https://crrev.com/9dbabee0d6c79ecf51ba5cc5edc7cd7d02f6b4fa/base/allocator/allocator_interception_mac.h
[add] https://crrev.com/9dbabee0d6c79ecf51ba5cc5edc7cd7d02f6b4fa/base/allocator/allocator_interception_mac.mm
[modify] https://crrev.com/9dbabee0d6c79ecf51ba5cc5edc7cd7d02f6b4fa/base/process/memory_mac.mm

Project Member

Comment 25 by bugdroid1@chromium.org, Jan 26 2017

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

commit d774b55241a0a1d4c7f0eba23592c4b5de4ce24c
Author: erikchen <erikchen@chromium.org>
Date: Thu Jan 26 20:56:24 2017

Refactor allocator_interception_mac to use {Store,Replace}ZoneFunctions.

This CL has no intended behavior change.

Pulling out the logic for {Store,Replace}ZoneFunctions allows the allocator shim
to easily integrate with the existing malloc interception code.

BUG= 665567 

Review-Url: https://codereview.chromium.org/2650363002
Cr-Original-Commit-Position: refs/heads/master@{#446255}
Committed: https://chromium.googlesource.com/chromium/src/+/50a11072abe7c112ed23956929b3ac4d0590f6ff
Review-Url: https://codereview.chromium.org/2650363002
Cr-Commit-Position: refs/heads/master@{#446440}

[modify] https://crrev.com/d774b55241a0a1d4c7f0eba23592c4b5de4ce24c/base/allocator/allocator_interception_mac.h
[modify] https://crrev.com/d774b55241a0a1d4c7f0eba23592c4b5de4ce24c/base/allocator/allocator_interception_mac.mm

Project Member

Comment 26 by bugdroid1@chromium.org, Jan 27 2017

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

commit 8cf26ee794a218d356d508521c80bf1e7520076b
Author: erikchen <erikchen@chromium.org>
Date: Fri Jan 27 17:08:34 2017

Remove unnecessary lines from chrome/app/framework.order.

The symbols __ZnwmPv and __ZdlPvS_ are no longer present in the final binary.

BUG= 665567 

Review-Url: https://codereview.chromium.org/2647343008
Cr-Commit-Position: refs/heads/master@{#446698}

[modify] https://crrev.com/8cf26ee794a218d356d508521c80bf1e7520076b/chrome/app/framework.order

Project Member

Comment 27 by bugdroid1@chromium.org, Jan 27 2017

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

commit 83d8f196d15bc31bfbce50ecc435016ba2a96bcf
Author: erikchen <erikchen@chromium.org>
Date: Fri Jan 27 22:37:54 2017

Fold //base/allocator:unified_allocator_shim into //base:base.

This CL is a refactor and has no intended behavior change.

There is no reason to have a separate source set for
//base/allocator:unified_allocator_shim, which introduces dependency cycles.

BUG= 665567 

Review-Url: https://codereview.chromium.org/2651043008
Cr-Commit-Position: refs/heads/master@{#446800}

[modify] https://crrev.com/83d8f196d15bc31bfbce50ecc435016ba2a96bcf/base/BUILD.gn
[modify] https://crrev.com/83d8f196d15bc31bfbce50ecc435016ba2a96bcf/base/allocator/BUILD.gn

Blocking: 686275
Project Member

Comment 29 by bugdroid1@chromium.org, Feb 2 2017

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

commit 0d0395aeb25b9ab1422ab547f976d049a14e192f
Author: erikchen <erikchen@chromium.org>
Date: Thu Feb 02 06:16:29 2017

Hook up allocator shim on mac.

This CL should have no behavioral effect, since the relevant code is not enabled
yet.

allocator_shim_override_mac_symbols.h intercepts heap allocations using the
newly introduced APIs in allocator_interception_mac.h and redirects them to the
allocator shim. This eventually forwards to AllocatorDispatch::default_dispatch,
which in turns calls the original malloc zone functions..

BUG= 665567 

Review-Url: https://codereview.chromium.org/2658723007
Cr-Commit-Position: refs/heads/master@{#447705}

[modify] https://crrev.com/0d0395aeb25b9ab1422ab547f976d049a14e192f/base/BUILD.gn
[modify] https://crrev.com/0d0395aeb25b9ab1422ab547f976d049a14e192f/base/allocator/allocator_interception_mac.h
[modify] https://crrev.com/0d0395aeb25b9ab1422ab547f976d049a14e192f/base/allocator/allocator_interception_mac.mm
[modify] https://crrev.com/0d0395aeb25b9ab1422ab547f976d049a14e192f/base/allocator/allocator_shim.cc
[modify] https://crrev.com/0d0395aeb25b9ab1422ab547f976d049a14e192f/base/allocator/allocator_shim.h
[modify] https://crrev.com/0d0395aeb25b9ab1422ab547f976d049a14e192f/base/allocator/allocator_shim_default_dispatch_to_glibc.cc
[modify] https://crrev.com/0d0395aeb25b9ab1422ab547f976d049a14e192f/base/allocator/allocator_shim_default_dispatch_to_linker_wrapped_symbols.cc
[add] https://crrev.com/0d0395aeb25b9ab1422ab547f976d049a14e192f/base/allocator/allocator_shim_default_dispatch_to_mac_zoned_malloc.cc
[add] https://crrev.com/0d0395aeb25b9ab1422ab547f976d049a14e192f/base/allocator/allocator_shim_default_dispatch_to_mac_zoned_malloc.h
[modify] https://crrev.com/0d0395aeb25b9ab1422ab547f976d049a14e192f/base/allocator/allocator_shim_default_dispatch_to_tcmalloc.cc
[modify] https://crrev.com/0d0395aeb25b9ab1422ab547f976d049a14e192f/base/allocator/allocator_shim_default_dispatch_to_winheap.cc
[add] https://crrev.com/0d0395aeb25b9ab1422ab547f976d049a14e192f/base/allocator/allocator_shim_override_mac_symbols.h
[modify] https://crrev.com/0d0395aeb25b9ab1422ab547f976d049a14e192f/base/allocator/allocator_shim_unittest.cc
[modify] https://crrev.com/0d0395aeb25b9ab1422ab547f976d049a14e192f/base/debug/thread_heap_usage_tracker.cc
[modify] https://crrev.com/0d0395aeb25b9ab1422ab547f976d049a14e192f/base/debug/thread_heap_usage_tracker_unittest.cc
[modify] https://crrev.com/0d0395aeb25b9ab1422ab547f976d049a14e192f/base/trace_event/malloc_dump_provider.cc
[modify] https://crrev.com/0d0395aeb25b9ab1422ab547f976d049a14e192f/build/config/allocator.gni

Project Member

Comment 31 by bugdroid1@chromium.org, Feb 2 2017

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

commit 89e4e70ff72647ccc29b884b0965112a7153a4cf
Author: erikchen <erikchen@chromium.org>
Date: Thu Feb 02 21:20:40 2017

Fix an ASAN compile error in AllocatorInterceptionMac.

BUG= 665567 

Review-Url: https://codereview.chromium.org/2674653004
Cr-Commit-Position: refs/heads/master@{#447841}

[modify] https://crrev.com/89e4e70ff72647ccc29b884b0965112a7153a4cf/base/allocator/allocator_interception_mac.mm

Project Member

Comment 32 by bugdroid1@chromium.org, Feb 6 2017

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

commit e9ffc9c6104f4cc7ff5e62a94869f111c645106a
Author: primiano <primiano@chromium.org>
Date: Mon Feb 06 19:52:44 2017

profiler: decouple ThreadHeapUsageTracker from allocator shim internals

Background:
-----------
Until crrev.com/2658723007, the allocator shim was initialized at
compile time and hence always ready to be used. After that CL,
the shim requires a run-time initialization. The code in
ThreadHeapUsageTracker currently assumes that the shim is initialized
by directly calling into the shim chain even before a malloc/free call
is intercepted.
There are two possible solutions to this:
1. Initialize the shim before ThreadHeapUsageTracker is initialized.
2. Prevent ThreadHeapUsageTracker from directly entering the shim.
(more discussion in https://codereview.chromium.org/2658723007/#msg53).

1. is a one liner but not particularly clean. ThreadHeapUsageTracker is
lazily initialized when a Thread (the main thread) is created (in the
TrackedObject construction). This would mean, in practice, putting
the shim initialization in tracked_objects.cc which is quite obscure.

This CL:
--------
The approach used here is 2. The existing code had already the
right sentinel logic to detect and prevent re-entrancy. This CL
is just extending that to the dtor and just using new/delete to
create the TLS object.
This allows to initialize the shim in a less obscure place (e.g.
content_main_runner).

BUG= 665567 , 644385 
TEST=Build with use_experimental_allocator_shim=true on Mac, run base_unittests

Review-Url: https://codereview.chromium.org/2675433002
Cr-Commit-Position: refs/heads/master@{#448360}

[modify] https://crrev.com/e9ffc9c6104f4cc7ff5e62a94869f111c645106a/base/debug/thread_heap_usage_tracker.cc

Blocking: 677302
Blocking: 698266
Status: Fixed (was: Assigned)

Sign in to add a comment