mac: Hook up allocator shim during app startup.

base/allocator/allocator_interception_mac.mm

Index: base/allocator/allocator_interception_mac.mm
diff --git a/base/allocator/allocator_interception_mac.mm b/base/allocator/allocator_interception_mac.mm
index be6c0fa4e9e7261012d2af6525d31fbee9986343..c5c34dacaf88c4b9ef95085e28692c00dbce9efd 100644
--- a/base/allocator/allocator_interception_mac.mm
+++ b/base/allocator/allocator_interception_mac.mm
@@ -27,6 +27,8 @@
 
 #include <new>
 
+#include "base/allocator/allocator_shim.h"
+#include "base/allocator/features.h"
 #include "base/logging.h"
 #include "base/mac/mac_util.h"
 #include "base/mac/mach_logging.h"
@@ -448,6 +450,10 @@ void InterceptAllocationsMac() {
 
   std::set_new_handler(oom_killer_new);

[Primiano Tucci (use gerrit), 2017/02/07 11:58:08]

(read this comment after the one below)
Honestly I think this is the misplaced thing that mislead you. I know that you
didn't touch this, but I think that this call should really happen on 
memory_mac.mm -> EnableTerminationOnOutOfMemory(), similarly to what happens in
Linux.
In other words, I think it would be cleaner if   memory_mac.mm  looked like
this:
EnableTerminationOnOutOfMemory() {
  // Step 1, enable OOM killer on C++ failures
  std::set_new_handler(oom_killer_new);

  // Step 2: enabled OOM killer on C-malloc failures for the default zone (if we
have a shim) 
  #if BUILDFLAG(USE_EXPERIMENTAL_ALLOCATOR_SHIM)
    allocator::SetCallNewHandlerOnMallocFailure(true);
  #endif

  // Step 3: enable OOM killer on all other malloc zones (or just "all" without
"other" if shim is disabled)
  allocator::InterceptAllocationsMac();   // (and this is the reason why in the
previous CL I was saying that this has a bad name now.. but we'll deal with
naming later)
}

Other than being cleaner, as a side effect this will look more consistent with
memory_linux.cc EnableTerminationOnOutOfMemory()

[erikchen, 2017/02/09 23:49:04]

Done.

 
+#if BUILDFLAG(USE_EXPERIMENTAL_ALLOCATOR_SHIM)
+  allocator::SetCallNewHandlerOnMallocFailure(true);

[Primiano Tucci (use gerrit), 2017/02/07 11:58:08]

why here and not in memory_mac.mm -> EnableTerminationOnOutOfMemory() ?
That would be consistent with what we do today in memory_linux.cc.

this is creating a logic dependency cycle. So far, allocator_interception_mac.mm
has been  a low level building block, which  knows how to deal with malloc
replacement stuff *
allocator_interception_mac.mm has no idea what the shim layer is. the shim layer
uses allocator_interception_mac.mm to implement malloc shimming on OSX.
Now with this line you are creating the following situation:
- the shim layer knows about allocator_interception_mac.mm and uses it to insert
itself in the malloc zone.
- allocator_interception_mac.mm knows about the shim and uses it to turn on the
suicide-on-oom flag
So if you do this, and look at the full picture, the end-to-end stacktrace will
look as follows (i hope that rietveld won't screw up my indentation ascii art
with non-monospace fonts):

content_main_runner.cc               ContentMainRunnerImpl::Initialize()
   allocator_shim.cc                 InitializeAllocatorShim()
     allocator_interception_mac.mm   ReplaceFunctionsForDefaultZone()
       allocator_shim.cc             SetCallNewHandlerOnMallocFailure()
   memory_mac.mm                     EnableTerminationOnOutOfMemory()
     allocator_interception_mac.mm   InterceptAllocationsMac()

(note the allocator_shim -> interception_mac -> allocator_shim sequence)

Instead what I was proposing is:

content_main_runner.cc               ContentMainRunnerImpl::Initialize()
   allocator_shim.cc                 InitializeAllocatorShim()
     allocator_interception_mac.mm   ReplaceFunctionsForDefaultZone()
   memory_mac.mm                     EnableTerminationOnOutOfMemory()
     allocator_interception_mac.mm   InterceptAllocationsMac()
     allocator_shim.cc               SetCallNewHandlerOnMallocFailure()

which looks a bit more cleaner to me.

* There is indeed a non-ideal thing in all this, the fact that the
suicide-on-OOM for the non-default zones happens in
allocator_interception_mac.mm while the one for the default zone happens in the
shim. but, as we discussed in the past, we have to live with it and the only
other option is refactoring all the shim to introduce the notions of zones
(which is probably going to create more headaches).

[erikchen, 2017/02/09 23:49:04]

Done.

+#endif
+
 #ifndef ADDRESS_SANITIZER
   // === Core Foundation CFAllocators ===