-
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:
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}
70f64a02