Handle failures allocating mapped local memory.

base/metrics/persistent_memory_allocator.cc

Index: base/metrics/persistent_memory_allocator.cc
diff --git a/base/metrics/persistent_memory_allocator.cc b/base/metrics/persistent_memory_allocator.cc
index dfa408f44d2be244e859727515c16d402c39ad78..1ea547e356eb8d9f2f0c644c491ee62db1b9c6b6 100644
--- a/base/metrics/persistent_memory_allocator.cc
+++ b/base/metrics/persistent_memory_allocator.cc
@@ -17,6 +17,7 @@
 #include "base/logging.h"
 #include "base/memory/shared_memory.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/metrics/sparse_histogram.h"
 
 namespace {
 
@@ -744,21 +745,39 @@ LocalPersistentMemoryAllocator::~LocalPersistentMemoryAllocator() {
 
 // static
 void* LocalPersistentMemoryAllocator::AllocateLocalMemory(size_t size) {
+  void* address;
+
 #if defined(OS_WIN)
-  void* address =
+  address =
       ::VirtualAlloc(nullptr, size, MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE);
-  DPCHECK(address);
-  return address;
+  if (!address) {
+    UMA_HISTOGRAM_SPARSE_SLOWLY(
+        "UMA.LocalPersistentMemoryAllocator.Failures.Win", ::GetLastError());
+  }
 #elif defined(OS_POSIX)
   // MAP_ANON is deprecated on Linux but MAP_ANONYMOUS is not universal on Mac.
   // MAP_SHARED is not available on Linux <2.4 but required on Mac.
-  void* address = ::mmap(nullptr, size, PROT_READ | PROT_WRITE,
-                         MAP_ANON | MAP_SHARED, -1, 0);
-  DPCHECK(MAP_FAILED != address);
-  return address;
+  address = ::mmap(nullptr, size, PROT_READ | PROT_WRITE,
+                   MAP_ANON | MAP_SHARED, -1, 0);
+  if (address == MAP_FAILED) {
+    UMA_HISTOGRAM_SPARSE_SLOWLY(
+        "UMA.LocalPersistentMemoryAllocator.Failures.Posix", errno);
+    address = nullptr;
+  }
 #else
 #error This architecture is not (yet) supported.
 #endif
+
+  if (!address) {
+    // As a last resort, just allocate the memory from the heap. This will
+    // achieve the same basic result but the acquired memory has to be
+    // explicitly zeroed and thus realized immediately (i.e. all pages added
+    // to the process now istead of when first accessed).

[Alexei Svitkine (slow), 2016/09/22 16:57:12]

Can you add a histogram to track how often this happens?

[bcwhite, 2016/09/22 17:25:59]

I tried that (see patch #1) but it means creating a histogram every time one of
these gets created and since these are used for testing, it messes with the
expected results of a dozen histogram tests.

+    address = ::malloc(size);
+    DPCHECK(address);
+    ::memset(address, 0, size);

[Alexei Svitkine (slow), 2016/09/22 16:57:12]

Nit: I don't think it's general convention to prefix non-Chrome code with ::
unless there's conflicts.

[bcwhite, 2016/09/22 17:25:59]

Done.

+  }
+  return address;
 }
 
 // static
@@ -766,10 +785,16 @@ void LocalPersistentMemoryAllocator::DeallocateLocalMemory(void* memory,
                                                            size_t size) {
 #if defined(OS_WIN)
   BOOL success = ::VirtualFree(memory, 0, MEM_DECOMMIT);
-  DPCHECK(success);
+  if (!success) {
+    // Must have been allocated by fallback allocator.
+    ::free(memory);
+  }
 #elif defined(OS_POSIX)
   int result = ::munmap(memory, size);
-  DPCHECK(0 == result);
+  if (result < 0) {
+    // Must have been allocated by fallback allocator.

[Alexei Svitkine (slow), 2016/09/22 16:57:12]

I don't think this is a safe assumption - e.g. if the above fails for some other
reason. I would add a boolean to explicitly track this.

[bcwhite, 2016/09/22 17:25:59]

I'd like to but can't because there is no place to store it. This method is
static because it's used to determine a construction parameter to the base class
meaning no way to safely write to member variables.

According to the man page, the only reason this call should fail is EINVAL which
is the case we're looking at.

+    ::free(memory);
+  }
 #else
 #error This architecture is not (yet) supported.
 #endif