• Benoît Lizé's avatar
    Reland "[windows] Release the Win32 address space reservation on OOM." · 70f64a02
    Benoît Lizé authored
    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: default avatarBenoit L <lizeb@chromium.org>
    Reviewed-by: default avatarWez <wez@chromium.org>
    Commit-Queue: Benoit L <lizeb@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#731754}
    70f64a02
memory_linux.cc 4.51 KB