From ecb5c783d7c35d6111ea506f2ad9617abcc72598 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Wed, 16 Dec 2020 12:13:51 -0800 Subject: [PATCH 1/2] refactor: remove last use of InternalCallbackScope --- patches/node/.patches | 2 -- .../fix_expose_internalcallbackscope.patch | 20 -------------- ...ixme_remove_async_id_assertion_check.patch | 27 ------------------- shell/app/node_main.cc | 10 ++----- 4 files changed, 2 insertions(+), 57 deletions(-) delete mode 100644 patches/node/fix_expose_internalcallbackscope.patch delete mode 100644 patches/node/fixme_remove_async_id_assertion_check.patch diff --git a/patches/node/.patches b/patches/node/.patches index 568d55d7178ce..3d794384d19cf 100644 --- a/patches/node/.patches +++ b/patches/node/.patches @@ -5,7 +5,6 @@ feat_add_uv_loop_watcher_queue_code.patch feat_initialize_asar_support.patch expose_get_builtin_module_function.patch fix_build_and_expose_inspector_agent.patch -fix_expose_internalcallbackscope.patch build_add_gn_build_files.patch fix_add_default_values_for_enable_lto_and_build_v8_with_gn_in.patch feat_add_new_built_with_electron_variable_to_config_gypi.patch @@ -13,7 +12,6 @@ feat_add_flags_for_low-level_hooks_and_exceptions.patch fix_expose_tracing_agent_and_use_tracing_tracingcontroller_instead.patch pass_all_globals_through_require.patch call_process_log_from_fallback_stream_on_windows.patch -fixme_remove_async_id_assertion_check.patch fixme_comment_trace_event_macro.patch fix_key_gen_apis_are_not_available_in_boringssl.patch build_modify_js2c_py_to_allow_injection_of_original-fs_and_custom_embedder_js.patch diff --git a/patches/node/fix_expose_internalcallbackscope.patch b/patches/node/fix_expose_internalcallbackscope.patch deleted file mode 100644 index 81a2b9cc0c7df..0000000000000 --- a/patches/node/fix_expose_internalcallbackscope.patch +++ /dev/null @@ -1,20 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: deepak1556 -Date: Sat, 6 Jan 2018 18:28:10 +0530 -Subject: fix: expose InternalCallbackScope - -This commit exposes InternalCallbackScope in order to allow us access to its internal flags. - -diff --git a/src/node_internals.h b/src/node_internals.h -index c1555b312e2f22e191d91d34a348d2e163d85b5b..084ecfa73657d1958d7552baa896e170934639c8 100644 ---- a/src/node_internals.h -+++ b/src/node_internals.h -@@ -209,7 +209,7 @@ v8::MaybeLocal InternalMakeCallback( - v8::Local argv[], - async_context asyncContext); - --class InternalCallbackScope { -+class NODE_EXTERN InternalCallbackScope { - public: - enum Flags { - kNoFlags = 0, diff --git a/patches/node/fixme_remove_async_id_assertion_check.patch b/patches/node/fixme_remove_async_id_assertion_check.patch deleted file mode 100644 index 7b16ad7b87d7d..0000000000000 --- a/patches/node/fixme_remove_async_id_assertion_check.patch +++ /dev/null @@ -1,27 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: Shelley Vohr -Date: Thu, 13 Sep 2018 09:08:10 -0700 -Subject: FIXME: remove async_id assertion check - -async hooks are hella broken in Electron. This was checking that they weren't, -but they are, so we just disabled the check. YOLO. - -diff --git a/src/api/callback.cc b/src/api/callback.cc -index 9f52c25cf0d9005c2e70b76eb52eae1bd15f0a53..e151871dc90b6c29dc3fc3db162e24baeb45923d 100644 ---- a/src/api/callback.cc -+++ b/src/api/callback.cc -@@ -117,12 +117,14 @@ void InternalCallbackScope::Close() { - perform_stopping_check(); - } - -+#if 0 // FIXME(codebytere): figure out why this check fails/causes crash - // Make sure the stack unwound properly. If there are nested MakeCallback's - // then it should return early and not reach this code. - if (env_->async_hooks()->fields()[AsyncHooks::kTotals]) { - CHECK_EQ(env_->execution_async_id(), 0); - CHECK_EQ(env_->trigger_async_id(), 0); - } -+#endif - - if (!tick_info->has_tick_scheduled() && !tick_info->has_rejection_to_warn()) { - return; diff --git a/shell/app/node_main.cc b/shell/app/node_main.cc index 70b146d6fdd08..f9da61b9f942b 100644 --- a/shell/app/node_main.cc +++ b/shell/app/node_main.cc @@ -243,14 +243,8 @@ int NodeMain(int argc, char* argv[]) { NodeDebugger node_debugger(env); node_debugger.Start(); - // TODO(codebytere): we should try to handle this upstream. - { - v8::HandleScope scope(isolate); - node::InternalCallbackScope callback_scope( - env, v8::Object::New(isolate), {1, 0}, - node::InternalCallbackScope::kSkipAsyncHooks); - node::LoadEnvironment(env); - } + v8::HandleScope scope(isolate); + node::LoadEnvironment(env); env->set_trace_sync_io(env->options()->trace_sync_io); From bef77c0c5300976590436978849bfb6d7da3e1a3 Mon Sep 17 00:00:00 2001 From: Electron Bot Date: Wed, 16 Dec 2020 20:24:59 +0000 Subject: [PATCH 2/2] update patches --- patches/node/fixme_comment_trace_event_macro.patch | 2 +- patches/node/override_existing_v8_reallocate.patch | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/patches/node/fixme_comment_trace_event_macro.patch b/patches/node/fixme_comment_trace_event_macro.patch index 9005f1890d3fb..3cc60241bb67a 100644 --- a/patches/node/fixme_comment_trace_event_macro.patch +++ b/patches/node/fixme_comment_trace_event_macro.patch @@ -7,7 +7,7 @@ This broke the build at some point. Does it still? We should probably remove this patch and find out! diff --git a/src/node_internals.h b/src/node_internals.h -index 084ecfa73657d1958d7552baa896e170934639c8..cb388f3a9f12949fd3ecb0406f7b550f4ca5ca7c 100644 +index c1555b312e2f22e191d91d34a348d2e163d85b5b..b92cc7edbfb60ec8182db5083f8b7aebd5c5da94 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -375,10 +375,11 @@ class TraceEventScope { diff --git a/patches/node/override_existing_v8_reallocate.patch b/patches/node/override_existing_v8_reallocate.patch index 11131206c2426..a69cfab47bb66 100644 --- a/patches/node/override_existing_v8_reallocate.patch +++ b/patches/node/override_existing_v8_reallocate.patch @@ -9,7 +9,7 @@ be overridden. This patch can be removed once the relevant version of V8 makes its way into Node.js. diff --git a/src/node_internals.h b/src/node_internals.h -index cb388f3a9f12949fd3ecb0406f7b550f4ca5ca7c..f4b5c9bb7058da2355204a7285e5f7cc70c4ffda 100644 +index b92cc7edbfb60ec8182db5083f8b7aebd5c5da94..294bed9175125bbd544c7aa7df4229d182ca442d 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -115,7 +115,7 @@ class NodeArrayBufferAllocator : public ArrayBufferAllocator {