fix: crash when destroying node env with pending promises

electron · Mar 15, 2022 · 4d92d7f · 4d92d7f
1 parent 4bdb50e
commit 4d92d7f
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 9 deletions.
diff --git 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
@@ -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());
 }