fix: use Node.js isolate setup logic in bindings (#24579)
* fix: use Node.js isolate setup logic in bindings * Flags should be more process-specific * Remove redundant isolate function setting * Remove old SetFatalErrorHandler call
This commit is contained in:
parent
f0953902db
commit
bcba4baa85
8 changed files with 81 additions and 85 deletions
|
@ -42,3 +42,4 @@ lib_src_switch_buffer_kmaxlength_to_size_t.patch
|
|||
update_tests_after_increasing_typed_array_size.patch
|
||||
darwin_work_around_clock_jumping_back_in_time.patch
|
||||
fix_do_not_register_the_esm_loader_in_renderer_processes.patch
|
||||
fix_allow_preventing_setpromiserejectcallback.patch
|
||||
|
|
|
@ -62,7 +62,7 @@ index c6ef9dc13ab6f1d1a778871a62a0a98a01d84ec6..222555831aa1bf0b7b29b4b46e235c98
|
|||
const CleanupHookCallback& cb) const {
|
||||
return std::hash<void*>()(cb.arg_);
|
||||
diff --git a/src/env.cc b/src/env.cc
|
||||
index c56a25f31e42aaf8bcbc0e3bb3db4f7aad0cf124..58f2372d664c4c6f8a513cb4206c24940891aa6d 100644
|
||||
index 657d711e539d81bfd03166bbaaae7f0b5db91a5f..02c5ba259c94bb160972005998007d66731d9dde 100644
|
||||
--- a/src/env.cc
|
||||
+++ b/src/env.cc
|
||||
@@ -30,7 +30,6 @@ using v8::ArrayBuffer;
|
||||
|
@ -73,7 +73,7 @@ index c56a25f31e42aaf8bcbc0e3bb3db4f7aad0cf124..58f2372d664c4c6f8a513cb4206c2494
|
|||
using v8::Function;
|
||||
using v8::FunctionTemplate;
|
||||
using v8::HandleScope;
|
||||
@@ -500,7 +499,6 @@ void Environment::InitializeLibuv(bool start_profiler_idle_notifier) {
|
||||
@@ -494,7 +493,6 @@ void Environment::InitializeLibuv(bool start_profiler_idle_notifier) {
|
||||
[](uv_async_t* async) {
|
||||
Environment* env = ContainerOf(
|
||||
&Environment::task_queues_async_, async);
|
||||
|
@ -81,7 +81,7 @@ index c56a25f31e42aaf8bcbc0e3bb3db4f7aad0cf124..58f2372d664c4c6f8a513cb4206c2494
|
|||
env->RunAndClearNativeImmediates();
|
||||
});
|
||||
uv_unref(reinterpret_cast<uv_handle_t*>(&idle_prepare_handle_));
|
||||
@@ -1127,25 +1125,6 @@ void Environment::RunWeakRefCleanup() {
|
||||
@@ -1121,25 +1119,6 @@ void Environment::RunWeakRefCleanup() {
|
||||
isolate()->ClearKeptObjects();
|
||||
}
|
||||
|
||||
|
@ -131,10 +131,10 @@ index b6e02a2910cd8fe5ff3a17d6e1a98b937323ae3a..c1966a9f55126bdd65d8c9d529d93497
|
|||
static int const kNodeContextTag;
|
||||
|
||||
diff --git a/src/node.h b/src/node.h
|
||||
index 709d03145e3d5acdb67502110917e8147c275c60..a279cc7cc6205907eb5f9c3f6237513b2354f6be 100644
|
||||
index 638a1a85b046ce4db303d532f7cf36cca2271db5..b9b11b4331bd3ae4a87f65758ee09af25222e19a 100644
|
||||
--- a/src/node.h
|
||||
+++ b/src/node.h
|
||||
@@ -323,8 +323,6 @@ struct IsolateSettings {
|
||||
@@ -322,8 +322,6 @@ struct IsolateSettings {
|
||||
v8::PromiseRejectCallback promise_reject_callback = nullptr;
|
||||
v8::AllowWasmCodeGenerationCallback
|
||||
allow_wasm_code_generation_callback = nullptr;
|
||||
|
|
|
@ -23,52 +23,20 @@ Environment on the V8 context of blink, so no new V8 context is created.
|
|||
|
||||
As a result, a renderer process may have multiple Node Environments in it.
|
||||
|
||||
diff --git a/src/env.cc b/src/env.cc
|
||||
index 657d711e539d81bfd03166bbaaae7f0b5db91a5f..c56a25f31e42aaf8bcbc0e3bb3db4f7aad0cf124 100644
|
||||
--- a/src/env.cc
|
||||
+++ b/src/env.cc
|
||||
@@ -410,6 +410,12 @@ Environment::Environment(IsolateData* isolate_data,
|
||||
// TODO(joyeecheung): deserialize when the snapshot covers the environment
|
||||
// properties.
|
||||
CreateProperties();
|
||||
+
|
||||
+ // TODO(addaleax): the per-isolate state should not be controlled by
|
||||
+ // a single Environment.
|
||||
+ if (g_standalone_mode) {
|
||||
+ isolate()->SetPromiseRejectCallback(task_queue::PromiseRejectCallback);
|
||||
+ }
|
||||
}
|
||||
|
||||
Environment::~Environment() {
|
||||
diff --git a/src/node.cc b/src/node.cc
|
||||
index 4ff7824b0011685289716d61b02427c3e264965d..2a21c6dfe74b97c0d4ca8608486f8f35c77cc831 100644
|
||||
index 4ff7824b0011685289716d61b02427c3e264965d..f2b0b1585a14eaf6ffdb69a28888b42a4928f36b 100644
|
||||
--- a/src/node.cc
|
||||
+++ b/src/node.cc
|
||||
@@ -122,6 +122,9 @@ using v8::Undefined;
|
||||
@@ -122,6 +122,8 @@ using v8::Undefined;
|
||||
using v8::V8;
|
||||
using v8::Value;
|
||||
|
||||
+bool g_standalone_mode = true;
|
||||
+bool g_upstream_node_mode = true;
|
||||
+
|
||||
namespace per_process {
|
||||
|
||||
// node_revert.h
|
||||
@@ -337,6 +340,13 @@ MaybeLocal<Value> Environment::RunBootstrapping() {
|
||||
|
||||
CHECK(!has_run_bootstrapping_code());
|
||||
|
||||
+ if (g_standalone_mode) {
|
||||
+ isolate()->AddMessageListener(errors::PerIsolateMessageListener);
|
||||
+ }
|
||||
+ if (g_upstream_node_mode) {
|
||||
+ isolate()->SetFatalErrorHandler(OnFatalError);
|
||||
+ }
|
||||
+
|
||||
if (BootstrapInternalLoaders().IsEmpty()) {
|
||||
return MaybeLocal<Value>();
|
||||
}
|
||||
@@ -733,7 +743,9 @@ int InitializeNodeWithArgs(std::vector<std::string>* argv,
|
||||
@@ -733,7 +735,9 @@ int InitializeNodeWithArgs(std::vector<std::string>* argv,
|
||||
binding::RegisterBuiltinModules();
|
||||
|
||||
// Make inherited handles noninheritable.
|
||||
|
@ -79,7 +47,7 @@ index 4ff7824b0011685289716d61b02427c3e264965d..2a21c6dfe74b97c0d4ca8608486f8f35
|
|||
|
||||
// Cache the original command line to be
|
||||
// used in diagnostic reports.
|
||||
@@ -767,6 +779,8 @@ int InitializeNodeWithArgs(std::vector<std::string>* argv,
|
||||
@@ -767,6 +771,8 @@ int InitializeNodeWithArgs(std::vector<std::string>* argv,
|
||||
if (exit_code != 0) return exit_code;
|
||||
}
|
||||
#endif
|
||||
|
@ -88,7 +56,7 @@ index 4ff7824b0011685289716d61b02427c3e264965d..2a21c6dfe74b97c0d4ca8608486f8f35
|
|||
|
||||
const int exit_code = ProcessGlobalArgs(argv,
|
||||
exec_argv,
|
||||
@@ -811,6 +825,7 @@ int InitializeNodeWithArgs(std::vector<std::string>* argv,
|
||||
@@ -811,6 +817,7 @@ int InitializeNodeWithArgs(std::vector<std::string>* argv,
|
||||
}
|
||||
per_process::metadata.versions.InitializeIntlVersions();
|
||||
#endif
|
||||
|
@ -97,15 +65,14 @@ index 4ff7824b0011685289716d61b02427c3e264965d..2a21c6dfe74b97c0d4ca8608486f8f35
|
|||
NativeModuleEnv::InitializeCodeCache();
|
||||
|
||||
diff --git a/src/node.h b/src/node.h
|
||||
index 886216e2cb533e7337bc4f6816e2de796f64f81e..19d5eff164a543a38b4c77f99c2f15c30a226f77 100644
|
||||
index 886216e2cb533e7337bc4f6816e2de796f64f81e..8378553f28671e4685b4ed20279b2be5d202e527 100644
|
||||
--- a/src/node.h
|
||||
+++ b/src/node.h
|
||||
@@ -211,6 +211,9 @@ namespace node {
|
||||
@@ -211,6 +211,8 @@ namespace node {
|
||||
|
||||
class IsolateData;
|
||||
class Environment;
|
||||
+// Whether node should open some low level hooks.
|
||||
+NODE_EXTERN extern bool g_standalone_mode;
|
||||
+NODE_EXTERN extern bool g_upstream_node_mode;
|
||||
|
||||
// TODO(addaleax): Officially deprecate this and replace it with something
|
||||
|
|
|
@ -0,0 +1,50 @@
|
|||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||
From: Shelley Vohr <shelley.vohr@gmail.com>
|
||||
Date: Wed, 15 Jul 2020 18:43:32 -0700
|
||||
Subject: fix: allow preventing SetPromiseRejectCallback
|
||||
|
||||
We do not want to use the promise rejection callback that Node.js uses,
|
||||
because it does not send PromiseRejectionEvents to the global script context.
|
||||
We need to use the one Blink already provides, and so we need to
|
||||
slightly augment IsolateSettings to allow for that.
|
||||
|
||||
diff --git a/src/api/environment.cc b/src/api/environment.cc
|
||||
index 21980987644c6e83029157785dea463070456c77..20d9f91dcf6b5def05a706cf3389d64e9edbcf3e 100644
|
||||
--- a/src/api/environment.cc
|
||||
+++ b/src/api/environment.cc
|
||||
@@ -241,9 +241,11 @@ void SetIsolateMiscHandlers(v8::Isolate* isolate, const IsolateSettings& s) {
|
||||
s.allow_wasm_code_generation_callback : AllowWasmCodeGenerationCallback;
|
||||
isolate->SetAllowWasmCodeGenerationCallback(allow_wasm_codegen_cb);
|
||||
|
||||
- auto* promise_reject_cb = s.promise_reject_callback ?
|
||||
- s.promise_reject_callback : task_queue::PromiseRejectCallback;
|
||||
- isolate->SetPromiseRejectCallback(promise_reject_cb);
|
||||
+ if (s.flags & SHOULD_SET_PROMISE_REJECTION_CALLBACK) {
|
||||
+ auto* promise_reject_cb = s.promise_reject_callback ?
|
||||
+ s.promise_reject_callback : task_queue::PromiseRejectCallback;
|
||||
+ isolate->SetPromiseRejectCallback(promise_reject_cb);
|
||||
+ }
|
||||
|
||||
if (s.flags & DETAILED_SOURCE_POSITIONS_FOR_PROFILING)
|
||||
v8::CpuProfiler::UseDetailedSourcePositionsForProfiling(isolate);
|
||||
diff --git a/src/node.h b/src/node.h
|
||||
index b9b11b4331bd3ae4a87f65758ee09af25222e19a..60ecc3bd3499c23b297bc11e7f052aede20520c2 100644
|
||||
--- a/src/node.h
|
||||
+++ b/src/node.h
|
||||
@@ -304,12 +304,14 @@ class NODE_EXTERN MultiIsolatePlatform : public v8::Platform {
|
||||
|
||||
enum IsolateSettingsFlags {
|
||||
MESSAGE_LISTENER_WITH_ERROR_LEVEL = 1 << 0,
|
||||
- DETAILED_SOURCE_POSITIONS_FOR_PROFILING = 1 << 1
|
||||
+ DETAILED_SOURCE_POSITIONS_FOR_PROFILING = 1 << 1,
|
||||
+ SHOULD_SET_PROMISE_REJECTION_CALLBACK = 1 << 2
|
||||
};
|
||||
|
||||
struct IsolateSettings {
|
||||
uint64_t flags = MESSAGE_LISTENER_WITH_ERROR_LEVEL |
|
||||
- DETAILED_SOURCE_POSITIONS_FOR_PROFILING;
|
||||
+ DETAILED_SOURCE_POSITIONS_FOR_PROFILING |
|
||||
+ SHOULD_SET_PROMISE_REJECTION_CALLBACK;
|
||||
v8::MicrotasksPolicy policy = v8::MicrotasksPolicy::kExplicit;
|
||||
|
||||
// Error handling callbacks
|
|
@ -20,7 +20,7 @@ index 09d71b34581268bfe17c3182029cb3949d857d48..60d30b7eff7329c4235024c315251072
|
|||
int thread_pool_size,
|
||||
node::tracing::TracingController* tracing_controller) {
|
||||
diff --git a/src/node.h b/src/node.h
|
||||
index 19d5eff164a543a38b4c77f99c2f15c30a226f77..709d03145e3d5acdb67502110917e8147c275c60 100644
|
||||
index 8378553f28671e4685b4ed20279b2be5d202e527..638a1a85b046ce4db303d532f7cf36cca2271db5 100644
|
||||
--- a/src/node.h
|
||||
+++ b/src/node.h
|
||||
@@ -116,6 +116,7 @@ namespace node {
|
||||
|
@ -31,7 +31,7 @@ index 19d5eff164a543a38b4c77f99c2f15c30a226f77..709d03145e3d5acdb67502110917e814
|
|||
class TracingController;
|
||||
|
||||
}
|
||||
@@ -387,6 +388,8 @@ NODE_EXTERN Environment* GetCurrentEnvironment(v8::Local<v8::Context> context);
|
||||
@@ -386,6 +387,8 @@ NODE_EXTERN Environment* GetCurrentEnvironment(v8::Local<v8::Context> context);
|
||||
// it returns nullptr.
|
||||
NODE_EXTERN MultiIsolatePlatform* GetMainThreadMultiIsolatePlatform();
|
||||
|
||||
|
|
|
@ -167,10 +167,6 @@ int NodeMain(int argc, char* argv[]) {
|
|||
feature_list->InitializeFromCommandLine("", "");
|
||||
base::FeatureList::SetInstance(std::move(feature_list));
|
||||
|
||||
// We do not want to double-set the error level and promise rejection
|
||||
// callback.
|
||||
node::g_standalone_mode = false;
|
||||
|
||||
// Explicitly register electron's builtin modules.
|
||||
NodeBindings::RegisterBuiltinModules();
|
||||
|
||||
|
|
|
@ -33,17 +33,6 @@
|
|||
|
||||
namespace electron {
|
||||
|
||||
namespace {
|
||||
|
||||
// Called when there is a fatal error in V8, we just crash the process here so
|
||||
// we can get the stack trace.
|
||||
void FatalErrorCallback(const char* location, const char* message) {
|
||||
LOG(ERROR) << "Fatal error in V8: " << location << " " << message;
|
||||
ElectronBindings::Crash();
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
ElectronBindings::ElectronBindings(uv_loop_t* loop) {
|
||||
uv_async_init(loop, &call_next_tick_async_, OnCallNextTick);
|
||||
call_next_tick_async_.data = this;
|
||||
|
@ -86,8 +75,6 @@ void ElectronBindings::BindProcess(v8::Isolate* isolate,
|
|||
|
||||
void ElectronBindings::BindTo(v8::Isolate* isolate,
|
||||
v8::Local<v8::Object> process) {
|
||||
isolate->SetFatalErrorHandler(FatalErrorCallback);
|
||||
|
||||
gin_helper::Dictionary dict(isolate, process);
|
||||
BindProcess(isolate, &dict, metrics_.get());
|
||||
|
||||
|
|
|
@ -221,13 +221,6 @@ void SetNodeOptions(base::Environment* env) {
|
|||
}
|
||||
}
|
||||
|
||||
bool AllowWasmCodeGenerationCallback(v8::Local<v8::Context> context,
|
||||
v8::Local<v8::String>) {
|
||||
v8::Local<v8::Value> wasm_code_gen = context->GetEmbedderData(
|
||||
node::ContextEmbedderIndex::kAllowWasmCodeGeneration);
|
||||
return wasm_code_gen->IsUndefined() || wasm_code_gen->IsTrue();
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
namespace electron {
|
||||
|
@ -308,7 +301,6 @@ bool NodeBindings::IsInitialized() {
|
|||
void NodeBindings::Initialize() {
|
||||
TRACE_EVENT0("electron", "NodeBindings::Initialize");
|
||||
// Open node's error reporting system for browser process.
|
||||
node::g_standalone_mode = browser_env_ == BrowserEnvironment::BROWSER;
|
||||
node::g_upstream_node_mode = false;
|
||||
|
||||
#if defined(OS_LINUX)
|
||||
|
@ -403,28 +395,31 @@ node::Environment* NodeBindings::CreateEnvironment(
|
|||
global.Delete("_noBrowserGlobals");
|
||||
}
|
||||
|
||||
node::IsolateSettings is;
|
||||
|
||||
if (browser_env_ == BrowserEnvironment::BROWSER) {
|
||||
// This policy requires that microtask checkpoints be explicitly invoked.
|
||||
// Node.js requires this.
|
||||
context->GetIsolate()->SetMicrotasksPolicy(v8::MicrotasksPolicy::kExplicit);
|
||||
// Node.js requires that microtask checkpoints be explicitly invoked.
|
||||
is.policy = v8::MicrotasksPolicy::kExplicit;
|
||||
} else {
|
||||
// Match Blink's behavior by allowing microtasks invocation to be controlled
|
||||
// by MicrotasksScope objects.
|
||||
context->GetIsolate()->SetMicrotasksPolicy(v8::MicrotasksPolicy::kScoped);
|
||||
is.policy = v8::MicrotasksPolicy::kScoped;
|
||||
|
||||
// We do not want to use Node.js' message listener as it interferes with
|
||||
// Blink's.
|
||||
is.flags &= ~node::IsolateSettingsFlags::MESSAGE_LISTENER_WITH_ERROR_LEVEL;
|
||||
|
||||
// We do not want to use the promise rejection callback that Node.js uses,
|
||||
// because it does not send PromiseRejectionEvents to the global script
|
||||
// context. We need to use the one Blink already provides.
|
||||
is.flags &=
|
||||
~node::IsolateSettingsFlags::SHOULD_SET_PROMISE_REJECTION_CALLBACK;
|
||||
}
|
||||
|
||||
// This needs to be called before the inspector is initialized.
|
||||
env->InitializeDiagnostics();
|
||||
|
||||
// Set the callback to invoke to check if wasm code generation should be
|
||||
// allowed.
|
||||
context->GetIsolate()->SetAllowWasmCodeGenerationCallback(
|
||||
AllowWasmCodeGenerationCallback);
|
||||
|
||||
// Generate more detailed source positions to code objects. This results in
|
||||
// better results when mapping profiling samples to script source.
|
||||
v8::CpuProfiler::UseDetailedSourcePositionsForProfiling(
|
||||
context->GetIsolate());
|
||||
node::SetIsolateUpForNode(context->GetIsolate(), is);
|
||||
|
||||
gin_helper::Dictionary process(context->GetIsolate(), env->process_object());
|
||||
process.SetReadOnly("type", process_type);
|
||||
|
|
Loading…
Reference in a new issue