From d987bee0073310c0bf114dd5e5c9db5e9c68ee59 Mon Sep 17 00:00:00 2001 From: yangllu Date: Fri, 7 Mar 2025 00:34:19 +0800 Subject: [PATCH] fix: javascript heap OOM is not raised (#45895) 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. --- patches/node/.patches | 1 + ...ror_callback_in_node_isolatesettings.patch | 42 +++++++++++++++++++ shell/common/node_bindings.cc | 29 +++++++++++++ 3 files changed, 72 insertions(+) create mode 100644 patches/node/feat_add_oom_error_callback_in_node_isolatesettings.patch diff --git a/patches/node/.patches b/patches/node/.patches index 58552a60de6f..8d0f15414399 100644 --- a/patches/node/.patches +++ b/patches/node/.patches @@ -44,3 +44,4 @@ test_make_eval_snapshot_tests_more_flexible.patch build_option_to_use_custom_inspector_protocol_path.patch fix_adjust_wpt_and_webidl_tests_for_enabled_float16array.patch chore_add_createexternalizabletwobytestring_to_globals.patch +feat_add_oom_error_callback_in_node_isolatesettings.patch diff --git a/patches/node/feat_add_oom_error_callback_in_node_isolatesettings.patch b/patches/node/feat_add_oom_error_callback_in_node_isolatesettings.patch new file mode 100644 index 000000000000..2e9c3f9840df --- /dev/null +++ b/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 +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 diff --git a/shell/common/node_bindings.cc b/shell/common/node_bindings.cc index c2dde178cf7b..56e8bc787674 100644 --- a/shell/common/node_bindings.cc +++ b/shell/common/node_bindings.cc @@ -10,6 +10,7 @@ #include #include +#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 context, v8::Local source) { // If we're running with contextIsolation enabled in the renderer process, @@ -688,6 +716,7 @@ std::shared_ptr 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.