Browse Source

fix: javascript heap OOM is not raised (#45911)

fix: javascript heap oom is not raised in node::OOMErrorHandler

node::OOMErrorHandler terminates the process directly without raising an
oom exception. To fix it, set an oom handler into node from electron.

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Yang Liu <[email protected]>
trop[bot] 1 month ago
parent
commit
10da0d694e

+ 1 - 0
patches/node/.patches

@@ -44,3 +44,4 @@ linux_try_preadv64_pwritev64_before_preadv_pwritev_4683.patch
 build_change_crdtp_protocoltypetraits_signatures_to_avoid_conflict.patch
 test_make_eval_snapshot_tests_more_flexible.patch
 build_option_to_use_custom_inspector_protocol_path.patch
+feat_add_oom_error_callback_in_node_isolatesettings.patch

+ 42 - 0
patches/node/feat_add_oom_error_callback_in_node_isolatesettings.patch

@@ -0,0 +1,42 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Yang Liu <[email protected]>
+Date: Wed, 5 Mar 2025 17:22:39 +0800
+Subject: feat: add oom_error_callback in node::IsolateSettings
+
+Expose v8::OOMErrorCallback to allow setting OOM error handler outside Node.js
+
+As described in this issue https://github.com/electron/electron/issues/45894,
+Electron fails to capture js heap oom because node::OOMErrorHandler just
+terminates the process without raising an exception.
+
+To fix this issue, provide the interface oom_error_callback to enable a
+custom oom error callback set from Electron.
+
+diff --git a/src/api/environment.cc b/src/api/environment.cc
+index 32fc075e97eebca6c47e796ac5308915746ffa2a..e72bee385865c7d34e9eea6b90c6d911d592f8af 100644
+--- a/src/api/environment.cc
++++ b/src/api/environment.cc
+@@ -241,7 +241,10 @@ void SetIsolateErrorHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
+   auto* fatal_error_cb = s.fatal_error_callback ?
+       s.fatal_error_callback : OnFatalError;
+   isolate->SetFatalErrorHandler(fatal_error_cb);
+-  isolate->SetOOMErrorHandler(OOMErrorHandler);
++
++  auto* oom_error_cb = s.oom_error_callback ?
++      s.oom_error_callback : OOMErrorHandler;
++  isolate->SetOOMErrorHandler(oom_error_cb);
+ 
+   if ((s.flags & SHOULD_NOT_SET_PREPARE_STACK_TRACE_CALLBACK) == 0) {
+     auto* prepare_stack_trace_cb = s.prepare_stack_trace_callback ?
+diff --git a/src/node.h b/src/node.h
+index afb26ec5690ccd65a3c36f8b8a1b2de416b9d843..98ad0ea649eaef43d1f5231f7bc4044e100e08d7 100644
+--- a/src/node.h
++++ b/src/node.h
+@@ -489,6 +489,7 @@ struct IsolateSettings {
+   v8::Isolate::AbortOnUncaughtExceptionCallback
+       should_abort_on_uncaught_exception_callback = nullptr;
+   v8::FatalErrorCallback fatal_error_callback = nullptr;
++  v8::OOMErrorCallback oom_error_callback = nullptr;
+   v8::PrepareStackTraceCallback prepare_stack_trace_callback = nullptr;
+ 
+   // Miscellaneous callbacks

+ 29 - 0
shell/common/node_bindings.cc

@@ -10,6 +10,7 @@
 #include <utility>
 #include <vector>
 
+#include "base/allocator/partition_allocator/src/partition_alloc/oom.h"
 #include "base/base_paths.h"
 #include "base/command_line.h"
 #include "base/containers/fixed_flat_set.h"
@@ -169,6 +170,33 @@ void V8FatalErrorCallback(const char* location, const char* message) {
   *zero = 0;
 }
 
+void V8OOMErrorCallback(const char* location, const v8::OOMDetails& details) {
+  const char* message =
+      details.is_heap_oom ? "Allocation failed - JavaScript heap out of memory"
+                          : "Allocation failed - process out of memory";
+  if (location) {
+    LOG(ERROR) << "OOM error in V8: " << location << " " << message;
+  } else {
+    LOG(ERROR) << "OOM error in V8: " << message;
+  }
+  if (details.detail) {
+    LOG(ERROR) << "OOM detail: " << details.detail;
+  }
+
+#if !IS_MAS_BUILD()
+  electron::crash_keys::SetCrashKey("electron.v8-oom.is_heap_oom",
+                                    std::to_string(details.is_heap_oom));
+  if (location) {
+    electron::crash_keys::SetCrashKey("electron.v8-oom.location", location);
+  }
+  if (details.detail) {
+    electron::crash_keys::SetCrashKey("electron.v8-oom.detail", details.detail);
+  }
+#endif
+
+  OOM_CRASH(0);
+}
+
 bool AllowWasmCodeGenerationCallback(v8::Local<v8::Context> context,
                                      v8::Local<v8::String> source) {
   // If we're running with contextIsolation enabled in the renderer process,
@@ -688,6 +716,7 @@ std::shared_ptr<node::Environment> NodeBindings::CreateEnvironment(
   // Use a custom fatal error callback to allow us to add
   // crash message and location to CrashReports.
   is.fatal_error_callback = V8FatalErrorCallback;
+  is.oom_error_callback = V8OOMErrorCallback;
 
   // We don't want to abort either in the renderer or browser processes.
   // We already listen for uncaught exceptions and handle them there.