Skip to content

Commit 71be38f

Browse files
authored
fix: revert enabling WASM trap handlers in all Node.js processes (electron#48973)
Revert "fix: enable wasm trap handlers in all Node.js processes (electron#48788)" This reverts commit ca0b46b.
1 parent 925966f commit 71be38f

5 files changed

Lines changed: 47 additions & 73 deletions

File tree

shell/browser/electron_browser_main_parts.cc

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@
6262
#include "shell/common/logging.h"
6363
#include "shell/common/node_bindings.h"
6464
#include "shell/common/node_includes.h"
65-
#include "shell/common/v8_util.h"
6665
#include "ui/base/idle/idle.h"
6766
#include "ui/base/l10n/l10n_util.h"
6867
#include "ui/base/ui_base_switches.h"
@@ -275,10 +274,6 @@ void ElectronBrowserMainParts::PostEarlyInitialization() {
275274
// Initialize field trials.
276275
InitializeFieldTrials();
277276

278-
if (base::FeatureList::IsEnabled(features::kWebAssemblyTrapHandler)) {
279-
electron::SetUpWebAssemblyTrapHandler();
280-
}
281-
282277
// Reinitialize logging now that the app has had a chance to set the app name
283278
// and/or user data directory.
284279
logging::InitElectronLogging(*base::CommandLine::ForCurrentProcess(),

shell/common/v8_util.cc

Lines changed: 0 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
#include <utility>
99
#include <vector>
1010

11-
#include "base/base_switches.h"
1211
#include "base/memory/raw_ptr.h"
1312
#include "gin/converter.h"
1413
#include "shell/common/api/electron_api_native_image.h"
@@ -18,15 +17,6 @@
1817
#include "ui/gfx/image/image_skia.h"
1918
#include "v8/include/v8.h"
2019

21-
#if BUILDFLAG(IS_LINUX) && (defined(ARCH_CPU_X86_64) || defined(ARCH_CPU_ARM64))
22-
#define ENABLE_WEB_ASSEMBLY_TRAP_HANDLER_LINUX
23-
#include "base/command_line.h"
24-
#include "base/debug/stack_trace.h"
25-
#include "components/crash/core/app/crashpad.h" // nogncheck
26-
#include "content/public/common/content_switches.h"
27-
#include "v8/include/v8-wasm-trap-handler-posix.h"
28-
#endif
29-
3020
namespace electron {
3121

3222
namespace {
@@ -250,51 +240,6 @@ v8::Local<v8::Value> DeserializeV8Value(v8::Isolate* isolate,
250240
return V8Deserializer(isolate, data).Deserialize();
251241
}
252242

253-
void SetUpWebAssemblyTrapHandler() {
254-
#if BUILDFLAG(IS_WIN)
255-
// On Windows we use the default trap handler provided by V8.
256-
v8::V8::EnableWebAssemblyTrapHandler(true);
257-
#elif BUILDFLAG(IS_MAC)
258-
// On macOS, Crashpad uses exception ports to handle signals in a
259-
// different process. As we cannot just pass a callback to this other
260-
// process, we ask V8 to install its own signal handler to deal with
261-
// WebAssembly traps.
262-
v8::V8::EnableWebAssemblyTrapHandler(true);
263-
#elif defined(ENABLE_WEB_ASSEMBLY_TRAP_HANDLER_LINUX)
264-
const bool crash_reporter_enabled =
265-
crash_reporter::GetHandlerSocket(nullptr, nullptr);
266-
267-
if (crash_reporter_enabled) {
268-
// If either --enable-crash-reporter or --enable-crash-reporter-for-testing
269-
// is enabled it should take care of signal handling for us, use the default
270-
// implementation which doesn't register an additional handler.
271-
v8::V8::EnableWebAssemblyTrapHandler(false);
272-
return;
273-
}
274-
275-
const bool use_v8_default_handler =
276-
base::CommandLine::ForCurrentProcess()->HasSwitch(
277-
::switches::kDisableInProcessStackTraces);
278-
279-
if (use_v8_default_handler) {
280-
// There is no signal handler yet, but it's okay if v8 registers one.
281-
v8::V8::EnableWebAssemblyTrapHandler(/*use_v8_signal_handler=*/true);
282-
return;
283-
}
284-
285-
if (base::debug::SetStackDumpFirstChanceCallback(
286-
v8::TryHandleWebAssemblyTrapPosix)) {
287-
// Crashpad and Breakpad are disabled, but the in-process stack dump
288-
// handlers are enabled, so set the callback on the stack dump handlers.
289-
v8::V8::EnableWebAssemblyTrapHandler(/*use_v8_signal_handler=*/false);
290-
return;
291-
}
292-
293-
// As the registration of the callback failed, we don't enable trap
294-
// handlers.
295-
#endif
296-
}
297-
298243
namespace util {
299244

300245
/**

shell/common/v8_util.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,6 @@ v8::Local<v8::Value> DeserializeV8Value(v8::Isolate* isolate,
3030
v8::Local<v8::Value> DeserializeV8Value(v8::Isolate* isolate,
3131
base::span<const uint8_t> data);
3232

33-
void SetUpWebAssemblyTrapHandler();
34-
3533
namespace util {
3634

3735
[[nodiscard]] base::span<uint8_t> as_byte_span(

shell/renderer/electron_renderer_client.cc

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
#include <algorithm>
88

9+
#include "base/base_switches.h"
910
#include "base/command_line.h"
1011
#include "base/containers/contains.h"
1112
#include "content/public/renderer/render_frame.h"
@@ -17,7 +18,6 @@
1718
#include "shell/common/node_includes.h"
1819
#include "shell/common/node_util.h"
1920
#include "shell/common/options_switches.h"
20-
#include "shell/common/v8_util.h"
2121
#include "shell/renderer/electron_render_frame_observer.h"
2222
#include "shell/renderer/web_worker_observer.h"
2323
#include "third_party/blink/public/common/web_preferences/web_preferences.h"
@@ -26,6 +26,13 @@
2626
#include "third_party/blink/renderer/core/execution_context/execution_context.h" // nogncheck
2727
#include "third_party/blink/renderer/core/frame/web_local_frame_impl.h" // nogncheck
2828

29+
#if BUILDFLAG(IS_LINUX) && (defined(ARCH_CPU_X86_64) || defined(ARCH_CPU_ARM64))
30+
#define ENABLE_WEB_ASSEMBLY_TRAP_HANDLER_LINUX
31+
#include "components/crash/core/app/crashpad.h" // nogncheck
32+
#include "content/public/common/content_switches.h"
33+
#include "v8/include/v8-wasm-trap-handler-posix.h"
34+
#endif
35+
2936
namespace electron {
3037

3138
ElectronRendererClient::ElectronRendererClient()
@@ -240,9 +247,45 @@ void ElectronRendererClient::WillDestroyWorkerContextOnWorkerThread(
240247
}
241248

242249
void ElectronRendererClient::SetUpWebAssemblyTrapHandler() {
243-
// content/renderer layer already takes care of the feature flag detection
244-
// so no need to check for features::kWebAssemblyTrapHandler here.
245-
electron::SetUpWebAssemblyTrapHandler();
250+
// See CL:5372409 - copied from ShellContentRendererClient.
251+
#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC)
252+
// Mac and Windows use the default implementation (where the default v8 trap
253+
// handler gets set up).
254+
ContentRendererClient::SetUpWebAssemblyTrapHandler();
255+
return;
256+
#elif defined(ENABLE_WEB_ASSEMBLY_TRAP_HANDLER_LINUX)
257+
const bool crash_reporter_enabled =
258+
crash_reporter::GetHandlerSocket(nullptr, nullptr);
259+
260+
if (crash_reporter_enabled) {
261+
// If either --enable-crash-reporter or --enable-crash-reporter-for-testing
262+
// is enabled it should take care of signal handling for us, use the default
263+
// implementation which doesn't register an additional handler.
264+
ContentRendererClient::SetUpWebAssemblyTrapHandler();
265+
return;
266+
}
267+
268+
const bool use_v8_default_handler =
269+
base::CommandLine::ForCurrentProcess()->HasSwitch(
270+
::switches::kDisableInProcessStackTraces);
271+
272+
if (use_v8_default_handler) {
273+
// There is no signal handler yet, but it's okay if v8 registers one.
274+
v8::V8::EnableWebAssemblyTrapHandler(/*use_v8_signal_handler=*/true);
275+
return;
276+
}
277+
278+
if (base::debug::SetStackDumpFirstChanceCallback(
279+
v8::TryHandleWebAssemblyTrapPosix)) {
280+
// Crashpad and Breakpad are disabled, but the in-process stack dump
281+
// handlers are enabled, so set the callback on the stack dump handlers.
282+
v8::V8::EnableWebAssemblyTrapHandler(/*use_v8_signal_handler=*/false);
283+
return;
284+
}
285+
286+
// As the registration of the callback failed, we don't enable trap
287+
// handlers.
288+
#endif // defined(ENABLE_WEB_ASSEMBLY_TRAP_HANDLER_LINUX)
246289
}
247290

248291
node::Environment* ElectronRendererClient::GetEnvironment(

shell/services/node/node_service.cc

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
#include "base/no_destructor.h"
1212
#include "base/process/process.h"
1313
#include "base/strings/utf_string_conversions.h"
14-
#include "content/public/common/content_features.h"
1514
#include "electron/mas.h"
1615
#include "net/base/network_change_notifier.h"
1716
#include "services/network/public/cpp/wrapper_shared_url_loader_factory.h"
@@ -23,7 +22,6 @@
2322
#include "shell/common/gin_helper/dictionary.h"
2423
#include "shell/common/node_bindings.h"
2524
#include "shell/common/node_includes.h"
26-
#include "shell/common/v8_util.h"
2725
#include "shell/services/node/parent_port.h"
2826

2927
#if !IS_MAS_BUILD()
@@ -132,11 +130,6 @@ void NodeService::Initialize(
132130
v8::Isolate* const isolate = js_env_->isolate();
133131
v8::HandleScope scope{isolate};
134132

135-
// Initialize after setting up the V8 isolate.
136-
if (base::FeatureList::IsEnabled(features::kWebAssemblyTrapHandler)) {
137-
electron::SetUpWebAssemblyTrapHandler();
138-
}
139-
140133
node_bindings_->Initialize(isolate, isolate->GetCurrentContext());
141134

142135
network_change_notifier_ = net::NetworkChangeNotifier::CreateIfNeeded(

0 commit comments

Comments
 (0)