Repository: electron/electron
Pull request: fix: revert enabling WASM trap handlers in all Node.js processes

#### Description of Change

Addresses https://github.com/electron/electron/issues/48956

Looking into the dmp file stacktrace attached to the above issue, the wasm trap handlers appear to be crashing shortly after app launch. This PR reverts the change while we investigate and find a forward fix.

This reverts commit ca0b46b4130a8b48ab0b402ea16efe1ce655044f and PR #48788 

#### Checklist
<!-- Remove items that do not apply. For completed items, change [ ] to [x]. -->

- [ ] PR description included and stakeholders cc'd
- [ ] `npm test` passes
- [ ] tests are [changed or added](https://github.com/electron/electron/blob/main/docs/development/testing.md)
- [ ] relevant API documentation, tutorials, and examples are updated and follow the [documentation style guide](https://github.com/electron/electron/blob/main/docs/development/style-guide.md)
- [ ] [PR release notes](https://github.com/electron/clerk/blob/main/README.md) describe the change in a way relevant to app developers, and are [capitalized, punctuated, and past tense](https://github.com/electron/clerk/blob/main/README.md#examples).

#### Release Notes

Notes: Fixed an issue where some apps would throw an exception on launch.


Changed files:
filename: shell/browser/electron_browser_main_parts.cc
patch: @@ -62,7 +62,6 @@
 #include "shell/common/logging.h"
 #include "shell/common/node_bindings.h"
 #include "shell/common/node_includes.h"
-#include "shell/common/v8_util.h"
 #include "ui/base/idle/idle.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/ui_base_switches.h"
@@ -275,10 +274,6 @@ void ElectronBrowserMainParts::PostEarlyInitialization() {
   // Initialize field trials.
   InitializeFieldTrials();
 
-  if (base::FeatureList::IsEnabled(features::kWebAssemblyTrapHandler)) {
-    electron::SetUpWebAssemblyTrapHandler();
-  }
-
   // Reinitialize logging now that the app has had a chance to set the app name
   // and/or user data directory.
   logging::InitElectronLogging(*base::CommandLine::ForCurrentProcess(),

filename: shell/common/v8_util.cc
patch: @@ -8,7 +8,6 @@
 #include <utility>
 #include <vector>
 
-#include "base/base_switches.h"
 #include "base/memory/raw_ptr.h"
 #include "gin/converter.h"
 #include "shell/common/api/electron_api_native_image.h"
@@ -18,15 +17,6 @@
 #include "ui/gfx/image/image_skia.h"
 #include "v8/include/v8.h"
 
-#if BUILDFLAG(IS_LINUX) && (defined(ARCH_CPU_X86_64) || defined(ARCH_CPU_ARM64))
-#define ENABLE_WEB_ASSEMBLY_TRAP_HANDLER_LINUX
-#include "base/command_line.h"
-#include "base/debug/stack_trace.h"
-#include "components/crash/core/app/crashpad.h"  // nogncheck
-#include "content/public/common/content_switches.h"
-#include "v8/include/v8-wasm-trap-handler-posix.h"
-#endif
-
 namespace electron {
 
 namespace {
@@ -250,51 +240,6 @@ v8::Local<v8::Value> DeserializeV8Value(v8::Isolate* isolate,
   return V8Deserializer(isolate, data).Deserialize();
 }
 
-void SetUpWebAssemblyTrapHandler() {
-#if BUILDFLAG(IS_WIN)
-  // On Windows we use the default trap handler provided by V8.
-  v8::V8::EnableWebAssemblyTrapHandler(true);
-#elif BUILDFLAG(IS_MAC)
-  // On macOS, Crashpad uses exception ports to handle signals in a
-  // different process. As we cannot just pass a callback to this other
-  // process, we ask V8 to install its own signal handler to deal with
-  // WebAssembly traps.
-  v8::V8::EnableWebAssemblyTrapHandler(true);
-#elif defined(ENABLE_WEB_ASSEMBLY_TRAP_HANDLER_LINUX)
-  const bool crash_reporter_enabled =
-      crash_reporter::GetHandlerSocket(nullptr, nullptr);
-
-  if (crash_reporter_enabled) {
-    // If either --enable-crash-reporter or --enable-crash-reporter-for-testing
-    // is enabled it should take care of signal handling for us, use the default
-    // implementation which doesn't register an additional handler.
-    v8::V8::EnableWebAssemblyTrapHandler(false);
-    return;
-  }
-
-  const bool use_v8_default_handler =
-      base::CommandLine::ForCurrentProcess()->HasSwitch(
-          ::switches::kDisableInProcessStackTraces);
-
-  if (use_v8_default_handler) {
-    // There is no signal handler yet, but it's okay if v8 registers one.
-    v8::V8::EnableWebAssemblyTrapHandler(/*use_v8_signal_handler=*/true);
-    return;
-  }
-
-  if (base::debug::SetStackDumpFirstChanceCallback(
-          v8::TryHandleWebAssemblyTrapPosix)) {
-    // Crashpad and Breakpad are disabled, but the in-process stack dump
-    // handlers are enabled, so set the callback on the stack dump handlers.
-    v8::V8::EnableWebAssemblyTrapHandler(/*use_v8_signal_handler=*/false);
-    return;
-  }
-
-  // As the registration of the callback failed, we don't enable trap
-  // handlers.
-#endif
-}
-
 namespace util {
 
 /**

filename: shell/common/v8_util.h
patch: @@ -30,8 +30,6 @@ v8::Local<v8::Value> DeserializeV8Value(v8::Isolate* isolate,
 v8::Local<v8::Value> DeserializeV8Value(v8::Isolate* isolate,
                                         base::span<const uint8_t> data);
 
-void SetUpWebAssemblyTrapHandler();
-
 namespace util {
 
 [[nodiscard]] base::span<uint8_t> as_byte_span(

filename: shell/renderer/electron_renderer_client.cc
patch: @@ -6,6 +6,7 @@
 
 #include <algorithm>
 
+#include "base/base_switches.h"
 #include "base/command_line.h"
 #include "base/containers/contains.h"
 #include "content/public/renderer/render_frame.h"
@@ -17,7 +18,6 @@
 #include "shell/common/node_includes.h"
 #include "shell/common/node_util.h"
 #include "shell/common/options_switches.h"
-#include "shell/common/v8_util.h"
 #include "shell/renderer/electron_render_frame_observer.h"
 #include "shell/renderer/web_worker_observer.h"
 #include "third_party/blink/public/common/web_preferences/web_preferences.h"
@@ -26,6 +26,13 @@
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"  // nogncheck
 #include "third_party/blink/renderer/core/frame/web_local_frame_impl.h"  // nogncheck
 
+#if BUILDFLAG(IS_LINUX) && (defined(ARCH_CPU_X86_64) || defined(ARCH_CPU_ARM64))
+#define ENABLE_WEB_ASSEMBLY_TRAP_HANDLER_LINUX
+#include "components/crash/core/app/crashpad.h"  // nogncheck
+#include "content/public/common/content_switches.h"
+#include "v8/include/v8-wasm-trap-handler-posix.h"
+#endif
+
 namespace electron {
 
 ElectronRendererClient::ElectronRendererClient()
@@ -240,9 +247,45 @@ void ElectronRendererClient::WillDestroyWorkerContextOnWorkerThread(
 }
 
 void ElectronRendererClient::SetUpWebAssemblyTrapHandler() {
-  // content/renderer layer already takes care of the feature flag detection
-  // so no need to check for features::kWebAssemblyTrapHandler here.
-  electron::SetUpWebAssemblyTrapHandler();
+// See CL:5372409 - copied from ShellContentRendererClient.
+#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC)
+  // Mac and Windows use the default implementation (where the default v8 trap
+  // handler gets set up).
+  ContentRendererClient::SetUpWebAssemblyTrapHandler();
+  return;
+#elif defined(ENABLE_WEB_ASSEMBLY_TRAP_HANDLER_LINUX)
+  const bool crash_reporter_enabled =
+      crash_reporter::GetHandlerSocket(nullptr, nullptr);
+
+  if (crash_reporter_enabled) {
+    // If either --enable-crash-reporter or --enable-crash-reporter-for-testing
+    // is enabled it should take care of signal handling for us, use the default
+    // implementation which doesn't register an additional handler.
+    ContentRendererClient::SetUpWebAssemblyTrapHandler();
+    return;
+  }
+
+  const bool use_v8_default_handler =
+      base::CommandLine::ForCurrentProcess()->HasSwitch(
+          ::switches::kDisableInProcessStackTraces);
+
+  if (use_v8_default_handler) {
+    // There is no signal handler yet, but it's okay if v8 registers one.
+    v8::V8::EnableWebAssemblyTrapHandler(/*use_v8_signal_handler=*/true);
+    return;
+  }
+
+  if (base::debug::SetStackDumpFirstChanceCallback(
+          v8::TryHandleWebAssemblyTrapPosix)) {
+    // Crashpad and Breakpad are disabled, but the in-process stack dump
+    // handlers are enabled, so set the callback on the stack dump handlers.
+    v8::V8::EnableWebAssemblyTrapHandler(/*use_v8_signal_handler=*/false);
+    return;
+  }
+
+  // As the registration of the callback failed, we don't enable trap
+  // handlers.
+#endif  // defined(ENABLE_WEB_ASSEMBLY_TRAP_HANDLER_LINUX)
 }
 
 node::Environment* ElectronRendererClient::GetEnvironment(

filename: shell/services/node/node_service.cc
patch: @@ -11,7 +11,6 @@
 #include "base/no_destructor.h"
 #include "base/process/process.h"
 #include "base/strings/utf_string_conversions.h"
-#include "content/public/common/content_features.h"
 #include "electron/mas.h"
 #include "net/base/network_change_notifier.h"
 #include "services/network/public/cpp/wrapper_shared_url_loader_factory.h"
@@ -23,7 +22,6 @@
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/node_bindings.h"
 #include "shell/common/node_includes.h"
-#include "shell/common/v8_util.h"
 #include "shell/services/node/parent_port.h"
 
 #if !IS_MAS_BUILD()
@@ -132,11 +130,6 @@ void NodeService::Initialize(
   v8::Isolate* const isolate = js_env_->isolate();
   v8::HandleScope scope{isolate};
 
-  // Initialize after setting up the V8 isolate.
-  if (base::FeatureList::IsEnabled(features::kWebAssemblyTrapHandler)) {
-    electron::SetUpWebAssemblyTrapHandler();
-  }
-
   node_bindings_->Initialize(isolate, isolate->GetCurrentContext());
 
   network_change_notifier_ = net::NetworkChangeNotifier::CreateIfNeeded(