Reland "[windows] Release the Win32 address space reservation on OOM."

Changes: only call a PartitionAlloc method when PartitionAlloc is
         enabled.


Original change's description:
> Revert "[windows] Release the Win32 address space reservation on OOM."
>
> This reverts commit db01ddb8.
>
> Reason for revert: Breaks linking on all cronet bots.
>
> ```
> ld.lld: error: undefined symbol: base::ReleaseReservation()
> >>> referenced by memory_linux.cc:48 (../../base/process/memory_linux.cc:48)
> >>>               base/memory_linux.o:(base::(anonymous namespace)::ReleaseReservationOrTerminate()) in archive obj/base/libbase.a
> clang: error: linker command failed with exit code 1 (use -v to see invocation)
> ```
>
> Bug: 1041737, 1028086, 1023804
>
> Original change's description:
> > [windows] Release the Win32 address space reservation on OOM.
> >
> > On Windows 32 bit, Chrome reserves a significant chunk of address space
> > when a renderer starts, to make large contiguous allocations easier to
> > fulfill. Once a memory allocation fails, the reservation is released,
> > then the allocation retried. This only works if *all* the allocators are
> > aware that this mechanism exists. Unfortunately, malloc() in particular
> > is not aware of this, meaning that if a large malloc() allocation bumps
> > into this issue first, then the renderer crashes.
> >
> > As this seems to be the case, from looking at crash reports (see linked
> > bugs for instance), malloc() should be made aware of this.
> >
> > On Windows, allocations use the allocator shim, and call the "new
> > handler", which should return a non-0 value when it is able to free some
> > memory. Currently, this new handler always crashes the process. We
> > update it to drop the reservation if it exists, and crash otherwise.
> >
> > This should hopefully solve some crashes we are seeing, especially some
> > related to video decoding, frequently allocating buffers of ~1MB, which
> > is large enough to trigger the issue, as seen in crash reports.
> >
> > This is not a complete fix, since it does not address all allocators in
> > a process, e.g. allocations by DLLs loaded by the process will not pass
> > through the new-handler hook. We do not (yet) see these as a significant
> > cause of crashes in practice though.
> >
> > Note that this CL also adds the reservation dropping behavior to Linux
> > builds as well. There is no address space reservation in actual Chrome
> > builds on Linux, but adding this allows to exercise the tests on Linux
> > builders as well, which is preferable.
> >
> > Bug: 1028086, 1023804
> > Change-Id: I12ad4aa0c7bf518c3cf1df93c966e8631a69c280
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1989747
> > Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
> > Reviewed-by: Benoit L <lizeb@chromium.org>
> > Reviewed-by: Wez <wez@chromium.org>
> > Commit-Queue: Benoit L <lizeb@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#730994}
>
> TBR=wez@chromium.org,brucedawson@chromium.org,lizeb@chromium.org
>
> Change-Id: I45bc4e480105548384291f97fc50df01c5e6ecc3
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 1028086, 1023804
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1999245
> Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
> Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#731042}

TBR=wez@chromium.org,nyquist@chromium.org,brucedawson@chromium.org,lizeb@chromium.org

Change-Id: Ie7cc4a04d816f5f8fc7de2cdc4ce0d003bbe7dfe
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1041737, 1028086, 1023804
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1999888Reviewed-by: Benoit L <lizeb@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#731754}

base/allocator/partition_allocator/page_allocator.cc

@@ -239,14 +239,21 @@ bool ReserveAddressSpace(size_t size) {
   return false;
 }
 
-void ReleaseReservation() {
+bool ReleaseReservation() {
   // To avoid deadlock, call only FreePages.
   subtle::SpinLock::Guard guard(GetReserveLock());
-  if (s_reservation_address != nullptr) {
+  if (!s_reservation_address)
+    return false;
+
   FreePages(s_reservation_address, s_reservation_size);
   s_reservation_address = nullptr;
   s_reservation_size = 0;
-  }
+  return true;
+}
+
+bool HasReservationForTesting() {
+  subtle::SpinLock::Guard guard(GetReserveLock());
+  return s_reservation_address != nullptr;
 }
 
 uint32_t GetAllocPageErrorCode() {

base/allocator/partition_allocator/page_allocator.h

@@ -182,7 +182,12 @@ BASE_EXPORT bool ReserveAddressSpace(size_t size);
 
 // Releases any reserved address space. |AllocPages| calls this automatically on
 // an allocation failure. External allocators may also call this on failure.
-BASE_EXPORT void ReleaseReservation();
+//
+// Returns true when an existing reservation was released.
+BASE_EXPORT bool ReleaseReservation();
+
+// Returns true if there is currently an address space reservation.
+BASE_EXPORT bool HasReservationForTesting();
 
 // Returns |errno| (POSIX) or the result of |GetLastError| (Windows) when |mmap|
 // (POSIX) or |VirtualAlloc| (Windows) fails.

base/process/memory.cc

@@ -2,9 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/process/memory.h"
 #include "base/debug/alias.h"
 #include "base/logging.h"
-#include "base/process/memory.h"
+#include "base/partition_alloc_buildflags.h"
+#if BUILDFLAG(USE_PARTITION_ALLOC)
+#include "base/allocator/partition_allocator/page_allocator.h"
+#endif
 #include "build/build_config.h"
 
 namespace base {
@@ -28,7 +32,7 @@ void TerminateBecauseOutOfMemory(size_t size) {
   OnNoMemory(size);
 }
 
-#endif
+#endif  // !defined(OS_WIN)
 
 // Defined in memory_mac.mm for Mac.
 #if !defined(OS_MACOSX)
@@ -49,6 +53,16 @@ bool UncheckedCalloc(size_t num_items, size_t size, void** result) {
   return true;
 }
 
+#endif  // defined(OS_MACOSX)
+
+namespace internal {
+bool ReleaseAddressSpaceReservation() {
+#if BUILDFLAG(USE_PARTITION_ALLOC)
+  return ReleaseReservation();
+#else
+  return false;
 #endif
+}
+}  // namespace internal
 
 }  // namespace base

base/process/memory.h

@@ -40,6 +40,12 @@ const int kMaxOomScore = 1000;
 BASE_EXPORT bool AdjustOOMScore(ProcessId process, int score);
 #endif
 
+namespace internal {
+// Returns true if address-space was released. Some configurations reserve part
+// of the process address-space for special allocations (e.g. WASM).
+bool ReleaseAddressSpaceReservation();
+}  // namespace internal
+
 #if defined(OS_WIN)
 namespace win {
 

base/process/memory_linux.cc

@@ -37,10 +37,18 @@ void OnNoMemorySize(size_t size) {
   LOG(FATAL) << "Out of memory.";
 }
 
-void OnNoMemory() {
+// NOINLINE as base::`anonymous namespace`::OnNoMemory() is recognized by the
+// crash server.
+NOINLINE void OnNoMemory() {
   OnNoMemorySize(0);
 }
 
+void ReleaseReservationOrTerminate() {
+  if (internal::ReleaseAddressSpaceReservation())
+    return;
+  OnNoMemory();
+}
+
 }  // namespace
 
 void EnableTerminationOnHeapCorruption() {
@@ -49,7 +57,7 @@ void EnableTerminationOnHeapCorruption() {
 
 void EnableTerminationOnOutOfMemory() {
   // Set the new-out of memory handler.
-  std::set_new_handler(&OnNoMemory);
+  std::set_new_handler(&ReleaseReservationOrTerminate);
   // If we're using glibc's allocator, the above functions will override
   // malloc and friends and make them die on out of memory.
 

base/process/memory_unittest.cc

@@ -9,9 +9,11 @@
 #include <stddef.h>
 
 #include <limits>
+#include <vector>
 
 #include "base/allocator/allocator_check.h"
 #include "base/allocator/buildflags.h"
+#include "base/allocator/partition_allocator/page_allocator.h"
 #include "base/compiler_specific.h"
 #include "base/debug/alias.h"
 #include "base/memory/aligned_memory.h"
@@ -497,6 +499,66 @@ TEST_F(OutOfMemoryTest, TerminateBecauseOutOfMemoryReportsAllocSize) {
 }
 #endif  // OS_WIN
 
+#if defined(ARCH_CPU_32_BITS) && (defined(OS_WIN) || defined(OS_LINUX))
+
+void TestAllocationsReleaseReservation(void* (*alloc_fn)(size_t),
+                                       void (*free_fn)(void*)) {
+  base::ReleaseReservation();
+  base::EnableTerminationOnOutOfMemory();
+
+  constexpr size_t kMiB = 1 << 20;
+  constexpr size_t kReservationSize = 512 * kMiB;  // MiB.
+
+  size_t reservation_size = kReservationSize;
+  while (!base::ReserveAddressSpace(reservation_size)) {
+    reservation_size -= 16 * kMiB;
+  }
+  ASSERT_TRUE(base::HasReservationForTesting());
+  ASSERT_GT(reservation_size, 0u);
+
+  // Allocate a large area at a time to bump into address space exhaustion
+  // before other limits. It is important not to do a larger allocation, to
+  // verify that we can allocate without removing the reservation. On the other
+  // hand, must be large enough to make the underlying implementation call
+  // mmap()/VirtualAlloc().
+  size_t allocation_size = reservation_size / 2;
+
+  std::vector<void*> areas;
+  // Pre-reserve the vector to make sure that we don't hit the address space
+  // limit while resizing the array.
+  areas.reserve(((2 * 4096 * kMiB) / allocation_size) + 1);
+
+  while (true) {
+    void* area = alloc_fn(allocation_size / 2);
+    ASSERT_TRUE(area);
+    areas.push_back(area);
+
+    // Working as intended, the allocation was successful, and the reservation
+    // was dropped instead of crashing.
+    //
+    // Meaning that the test is either successful, or crashes.
+    if (!base::HasReservationForTesting())
+      break;
+  }
+
+  EXPECT_GE(areas.size(), 2u)
+      << "Should be able to allocate without releasing the reservation";
+
+  for (void* ptr : areas)
+    free_fn(ptr);
+}
+
+TEST_F(OutOfMemoryHandledTest, MallocReleasesReservation) {
+  TestAllocationsReleaseReservation(malloc, free);
+}
+
+TEST_F(OutOfMemoryHandledTest, NewReleasesReservation) {
+  TestAllocationsReleaseReservation(
+      [](size_t size) { return static_cast<void*>(new char[size]); },
+      [](void* ptr) { delete[] static_cast<char*>(ptr); });
+}
+#endif  // defined(ARCH_CPU_32_BITS) && (defined(OS_WIN) || defined(OS_LINUX))
+
 // TODO(b.kelemen): make UncheckedMalloc and UncheckedCalloc work
 // on Windows as well.
 TEST_F(OutOfMemoryHandledTest, UncheckedMalloc) {

base/process/memory_win.cc

@@ -43,7 +43,7 @@ namespace {
 #pragma warning(push)
 #pragma warning(disable: 4702)  // Unreachable code after the _exit.
 
-NOINLINE int OnNoMemory(size_t size) {
+[[noreturn]] NOINLINE int OnNoMemory(size_t size) {
   // Kill the process. This is important for security since most of code
   // does not check the result of memory allocation.
   // https://msdn.microsoft.com/en-us/library/het71c37.aspx
@@ -54,11 +54,18 @@ NOINLINE int OnNoMemory(size_t size) {
 
   // Safety check, make sure process exits here.
   _exit(win::kOomExceptionCode);
-  return 0;
 }
 
 #pragma warning(pop)
 
+// Return a non-0 value to retry the allocation.
+int ReleaseReservationOrTerminate(size_t size) {
+  constexpr int kRetryAllocation = 1;
+  if (internal::ReleaseAddressSpaceReservation())
+    return kRetryAllocation;
+  OnNoMemory(size);
+}
+
 }  // namespace
 
 void TerminateBecauseOutOfMemory(size_t size) {
@@ -71,8 +78,9 @@ void EnableTerminationOnHeapCorruption() {
 }
 
 void EnableTerminationOnOutOfMemory() {
-  _set_new_handler(&OnNoMemory);
-  _set_new_mode(1);
+  constexpr int kCallNewHandlerOnAllocationFailure = 1;
+  _set_new_handler(&ReleaseReservationOrTerminate);
+  _set_new_mode(kCallNewHandlerOnAllocationFailure);
 }
 
 // Implemented using a weak symbol.