Browse Source

chore: add process memory limit details in parition_alloc oom handler (#44512)

chore: backport b5b8e05a8 from chromium
Robo 5 months ago
parent
commit
fb6faaab69

+ 1 - 0
patches/chromium/.patches

@@ -135,3 +135,4 @@ feat_allow_usage_of_sccontentsharingpicker_on_supported_platforms.patch
 feat_allow_-4_as_a_macos_screen_share_id.patch
 fix_software_compositing_infinite_loop.patch
 cherry-pick-8c4edae5e34d.patch
+oom_add_commit_limit_available_to_oom_exception_arguments.patch

+ 4 - 5
patches/chromium/cherry-pick-8c4edae5e34d.patch

@@ -1,7 +1,7 @@
-From 8c4edae5e34dbe82ebaaf9596710800ac524258a Mon Sep 17 00:00:00 2001
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
 From: Justin Lulejian <[email protected]>
 Date: Fri, 18 Oct 2024 21:34:12 +0000
-Subject: [PATCH] [M130][Extensions][ServiceWorker] Skip worker for isolated world module fetch
+Subject: Skip worker for isolated world module fetch
 
 Before this change, an isolated world (e.g. extension content script,
 but also others) could dynamically import a script from an accessible
@@ -27,13 +27,12 @@ Owners-Override: Daniel Yip <[email protected]>
 Bot-Commit: Rubber Stamper <[email protected]>
 Cr-Commit-Position: refs/branch-heads/6723@{#1432}
 Cr-Branched-From: 985f2961df230630f9cbd75bd6fe463009855a11-refs/heads/main@{#1356013}
----
 
 diff --git a/third_party/blink/renderer/core/loader/modulescript/module_script_loader.cc b/third_party/blink/renderer/core/loader/modulescript/module_script_loader.cc
-index 9fad30c7..b83416e 100644
+index f32bdc144f26a1ea5da2dad9ac0c8a782b0b2ff9..21e96096f1b1e3279e6a9d9687464045a9dd4288 100644
 --- a/third_party/blink/renderer/core/loader/modulescript/module_script_loader.cc
 +++ b/third_party/blink/renderer/core/loader/modulescript/module_script_loader.cc
-@@ -153,12 +153,20 @@
+@@ -153,12 +153,20 @@ void ModuleScriptLoader::FetchInternal(
    url_ = module_request.Url();
  #endif
  

+ 2 - 2
patches/chromium/fix_software_compositing_infinite_loop.patch

@@ -10,10 +10,10 @@ process CompositingModeFallbackToSoftware IPC to disable GPU compositing.
 https://issues.chromium.org/345275130
 
 diff --git a/third_party/blink/renderer/platform/widget/compositing/layer_tree_view.cc b/third_party/blink/renderer/platform/widget/compositing/layer_tree_view.cc
-index 9497ab591864295231db47fdc526e59935e6aa31..635fa41649db41800f99da2683cc955b2e1e935f 100644
+index 3bbaa7dafcff1c88badaec0dbf14fd3320130398..8db903f3f343ed8723ad8adcb978b01fc27c6d9e 100644
 --- a/third_party/blink/renderer/platform/widget/compositing/layer_tree_view.cc
 +++ b/third_party/blink/renderer/platform/widget/compositing/layer_tree_view.cc
-@@ -374,9 +374,13 @@ void LayerTreeView::DidFailToInitializeLayerTreeFrameSink() {
+@@ -375,9 +375,13 @@ void LayerTreeView::DidFailToInitializeLayerTreeFrameSink() {
    // unable to be killed after Chrome is closed.
    // https://issues.chromium.org/336164423
    if (!Platform::Current()->IsGpuRemoteDisconnected()) {

+ 86 - 0
patches/chromium/oom_add_commit_limit_available_to_oom_exception_arguments.patch

@@ -0,0 +1,86 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Francois Doray <[email protected]>
+Date: Mon, 16 Sep 2024 15:57:01 +0000
+Subject: [oom] Add commit limit/available to OOM exception arguments
+ (Windows).
+
+~25% of OOM crashes [1] occur for an allocation <10MB when the
+"system commit remaining" [2] is >1GB. This CL adds the
+"process commit limit" and "process commit available" value from
+MEMORYSTATUSEX to the OOM exception arguments, which will allow
+verifying if the "process commit limit/remaining" is lower than
+the "system commit limit/remaining" when these OOM crashes occur.
+Depending on what we find, we might add these values to the
+stability_report.ProcessState proto in the future.
+
+[1] go/win-ooms-alloc-less-10mb-commit-remaining-more-1gb-126+
+    go/all-win-ooms-m126+
+[2] Computed asynchronously by crashpad from PERFORMANCE_INFORMATION:
+    (CommitLimit - CommitTotal)
+
+Change-Id: I1d6b34ce15fad46516a4e34a9507c341bfb81047
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5856490
+Auto-Submit: Francois Pierre Doray <[email protected]>
+Reviewed-by: Benoit Lize <[email protected]>
+Reviewed-by: Greg Thompson <[email protected]>
+Commit-Queue: Francois Pierre Doray <[email protected]>
+Cr-Commit-Position: refs/heads/main@{#1355900}
+
+diff --git a/base/allocator/partition_allocator/src/partition_alloc/oom.cc b/base/allocator/partition_allocator/src/partition_alloc/oom.cc
+index c0488caa98badb7f6fcef5d8051c41c4306eb316..9b78dfb817139c5f1a08d2e7c600d76c96fc4b85 100644
+--- a/base/allocator/partition_allocator/src/partition_alloc/oom.cc
++++ b/base/allocator/partition_allocator/src/partition_alloc/oom.cc
+@@ -28,21 +28,47 @@ namespace internal {
+ // partition_alloc::internal::base::internal::OnNoMemoryInternal
+ PA_NOINLINE void OnNoMemoryInternal(size_t size) {
+   g_oom_size = size;
++  size_t tmp_size = size;
++  internal::base::debug::Alias(&tmp_size);
++
+ #if PA_BUILDFLAG(IS_WIN)
++  // Create an exception vector with:
++  // [0] the size of the allocation, in bytes
++  // [1] "current committed memory limit for the system or the current process,
++  //     whichever is smaller, in bytes"
++  // [2] "maximum amount of memory the current process can commit, in bytes"
++  //
++  // Citations from
++  // https://learn.microsoft.com/en-us/windows/win32/api/sysinfoapi/ns-sysinfoapi-memorystatusex
++  //
++  // System commit constraints (which may be different from the process commit
++  // constraints) are in the stability_report.SystemMemoryState.WindowsMemory
++  // proto attached to crash reports.
++  //
++  // Note: Both the process commit constraints in the exception vector and the
++  // system commit constraints in the proto are collected *after* the OOM and
++  // may therefore not reflect the state at the time of the OOM (e.g. another
++  // process may have exited or the page file may have been resized).
++  constexpr size_t kInvalid = std::numeric_limits<ULONG_PTR>::max();
++  ULONG_PTR exception_args[] = {size, kInvalid, kInvalid};
++
++  MEMORYSTATUSEX memory_status = {};
++  memory_status.dwLength = sizeof(memory_status);
++  if (::GlobalMemoryStatusEx(&memory_status) != 0) {
++    exception_args[1] = memory_status.ullTotalPageFile;
++    exception_args[2] = memory_status.ullAvailPageFile;
++  }
++  internal::base::debug::Alias(&memory_status);
++
+   // 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
+-  // Pass the size of the failed request in an exception argument.
+-  ULONG_PTR exception_args[] = {size};
++  // Documentation: https://msdn.microsoft.com/en-us/library/het71c37.aspx
+   ::RaiseException(win::kOomExceptionCode, EXCEPTION_NONCONTINUABLE,
+                    std::size(exception_args), exception_args);
+ 
+   // Safety check, make sure process exits here.
+   _exit(win::kOomExceptionCode);
+ #else
+-  size_t tmp_size = size;
+-  internal::base::debug::Alias(&tmp_size);
+-
+   // Note: Don't add anything that may allocate here. Depending on the
+   // allocator, this may be called from within the allocator (e.g. with
+   // PartitionAlloc), and would deadlock as our locks are not recursive.

+ 2 - 2
patches/chromium/printing.patch

@@ -950,10 +950,10 @@ index e89fd87753bad3c5663fa53f8dcc4542e7e307e5..2b433a0705234af6f9808ee741a9795d
  
  base::FilePath GetCanonicalPath(const base::FilePath& path) {
 diff --git a/ui/gtk/gtk_util.cc b/ui/gtk/gtk_util.cc
-index d86fbcf969f2fa0d176ead903703ab612e5464c2..6b963ea8401d20e655d068a69105586814bab320 100644
+index 2bff2a36b980f0e96b66a651825c4b7c12c07156..ac7029444cc468ac8b1959e58bfd3520b077ef13 100644
 --- a/ui/gtk/gtk_util.cc
 +++ b/ui/gtk/gtk_util.cc
-@@ -227,9 +227,13 @@ aura::Window* GetAuraTransientParent(GtkWidget* dialog) {
+@@ -222,9 +222,13 @@ aura::Window* GetAuraTransientParent(GtkWidget* dialog) {
  }
  
  void ClearAuraTransientParent(GtkWidget* dialog, aura::Window* parent) {

+ 1 - 1
patches/dawn/tint_validate_that_align_is_large_enough.patch

@@ -1,7 +1,7 @@
 From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
 From: James Price <[email protected]>
 Date: Mon, 28 Oct 2024 16:57:46 +0000
-Subject: [tint] Validate that `@align()` is large enough
+Subject: Validate that `@align()` is large enough
 MIME-Version: 1.0
 Content-Type: text/plain; charset=UTF-8
 Content-Transfer-Encoding: 8bit

+ 1 - 2
patches/v8/cherry-pick-153d4e84e5d1.patch

@@ -1,8 +1,7 @@
 From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
 From: Andreas Haas <[email protected]>
 Date: Thu, 10 Oct 2024 13:56:42 +0200
-Subject: [13.0][wasm] Don't tier up wrapper if signature depends on other
- instance
+Subject: Don't tier up wrapper if signature depends on other instance
 
 The wasm-to-js wrapper tierup currently does not handle signatures with
 indexed types correctly if the WebAssembly instance from which the

+ 4 - 5
patches/v8/cherry-pick-d9893f4856af.patch

@@ -1,7 +1,7 @@
-From d9893f4856af26e78ba5021063ee2b1c61a3023b Mon Sep 17 00:00:00 2001
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
 From: Thibaud Michaud <[email protected]>
 Date: Thu, 10 Oct 2024 18:54:04 +0200
-Subject: [PATCH] Merged: [wasm] Fix default externref/exnref reference
+Subject: Merged: [wasm] Fix default externref/exnref reference
 
 - The default nullexternref should be null instead of undefined
 - The default exnref/nullexnref should be null instead of wasm_null
@@ -17,13 +17,12 @@ Commit-Queue: Thibaud Michaud <[email protected]>
 Cr-Commit-Position: refs/branch-heads/13.0@{#35}
 Cr-Branched-From: 4be854bd71ea878a25b236a27afcecffa2e29360-refs/heads/13.0.245@{#1}
 Cr-Branched-From: 1f5183f7ad6cca21029fd60653d075730c644432-refs/heads/main@{#96103}
----
 
 diff --git a/src/wasm/wasm-js.cc b/src/wasm/wasm-js.cc
-index f8549cb..7c07f1e 100644
+index 32bbbce5838a65c62d419bb7b79a36827d7b97f2..0d24a204f68a64bb65c7b102fe52571e23fe29a0 100644
 --- a/src/wasm/wasm-js.cc
 +++ b/src/wasm/wasm-js.cc
-@@ -1322,9 +1322,12 @@
+@@ -1314,9 +1314,12 @@ i::Handle<i::HeapObject> DefaultReferenceValue(i::Isolate* isolate,
    DCHECK(type.is_object_reference());
    // Use undefined for JS type (externref) but null for wasm types as wasm does
    // not know undefined.