From 4d92d7fb8615e150f6052339518ddd34c503db68 Mon Sep 17 00:00:00 2001 From: deepak1556 Date: Tue, 15 Mar 2022 21:20:23 +0900 Subject: [PATCH] fix: crash when destroying node env with pending promises --- shell/renderer/electron_renderer_client.cc | 19 ++++++++++++------- shell/renderer/web_worker_observer.cc | 9 +++++++-- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/shell/renderer/electron_renderer_client.cc b/shell/renderer/electron_renderer_client.cc index f24c8073935fe..2c80080172c1f 100644 --- a/shell/renderer/electron_renderer_client.cc +++ b/shell/renderer/electron_renderer_client.cc @@ -151,17 +151,22 @@ void ElectronRendererClient::WillReleaseScriptContext( if (env == node_bindings_->uv_env()) node_bindings_->set_uv_env(nullptr); - // Destroy the node environment. We only do this if node support has been - // enabled for sub-frames to avoid a change-of-behavior / introduce crashes - // for existing users. - // We also do this if we have disable electron site instance overrides to - // avoid memory leaks - auto prefs = render_frame->GetBlinkPreferences(); - gin_helper::MicrotasksScope microtasks_scope(env->isolate()); + // Destroying the node environment will also run the uv loop, + // Node.js expects `kExplicit` microtasks policy and will run microtasks + // checkpoints after every call into JavaScript. Since we use a different + // policy in the renderer - switch to `kExplicit` and then drop back to the + // previous policy value. + v8::Isolate* isolate = context->GetIsolate(); + auto old_policy = isolate->GetMicrotasksPolicy(); + DCHECK_EQ(v8::MicrotasksScope::GetCurrentDepth(isolate), 0); + isolate->SetMicrotasksPolicy(v8::MicrotasksPolicy::kExplicit); + node::FreeEnvironment(env); if (env == node_bindings_->uv_env()) node::FreeIsolateData(node_bindings_->isolate_data()); + isolate->SetMicrotasksPolicy(old_policy); + // ElectronBindings is tracking node environments. electron_bindings_->EnvironmentDestroyed(env); } diff --git a/shell/renderer/web_worker_observer.cc b/shell/renderer/web_worker_observer.cc index 287be9ead13f0..ca929bf9e4bb9 100644 --- a/shell/renderer/web_worker_observer.cc +++ b/shell/renderer/web_worker_observer.cc @@ -37,8 +37,13 @@ WebWorkerObserver::WebWorkerObserver() WebWorkerObserver::~WebWorkerObserver() { lazy_tls.Pointer()->Set(nullptr); - gin_helper::MicrotasksScope microtasks_scope( - node_bindings_->uv_env()->isolate()); + // Destroying the node environment will also run the uv loop, + // Node.js expects `kExplicit` microtasks policy and will run microtasks + // checkpoints after every call into JavaScript. Since we use a different + // policy in the renderer - switch to `kExplicit` + v8::Isolate* isolate = node_bindings_->uv_env()->isolate(); + DCHECK_EQ(v8::MicrotasksScope::GetCurrentDepth(isolate), 0); + isolate->SetMicrotasksPolicy(v8::MicrotasksPolicy::kExplicit); node::FreeEnvironment(node_bindings_->uv_env()); node::FreeIsolateData(node_bindings_->isolate_data()); }