fix: defer Wrappable destruction in SecondWeakCallback to a posted task (#50695)

V8's second-pass weak callbacks run inside a
DisallowJavascriptExecutionScope: they may touch the V8 API but must
not invoke JS, directly or indirectly. Several Electron Wrappables
(WebContents in particular) emit JS events from their destructors,
so deleting synchronously inside SecondWeakCallback can crash with
"Invoke in DisallowJavascriptExecutionScope" when GC happens to
collect the JS wrapper during a foreground GC task — typically during
shutdown's uv_run drain after a leaked WebContentsView.

This was previously latent and timing-dependent (electron/electron#47420,
electron/electron#45416, podman-desktop/podman-desktop#12409). The
esbuild migration's keepNames option (which wraps every function/class
with an Object.defineProperty call) shifted heap layout enough to make
the spec/fixtures/crash-cases/webcontentsview-create-leak-exit case
reliably reproduce it on every run, giving a clean signal for the fix.

Both WrappableBase and DeprecatedWrappableBase SecondWeakCallback now
post the deletion via base::SequencedTaskRunner::GetCurrentDefault()
so the destructor (and any Emit it does) runs once V8 has left the GC
scope. Falls back to synchronous deletion if no task runner is
available (early/late process lifetime).

Fixes electron/electron#47420.

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Sam Attard <sattard@anthropic.com>

shell/common/gin_helper/wrappable.cc

@@ -4,6 +4,7 @@
 
 #include "shell/common/gin_helper/wrappable.h"
 
+#include "base/task/sequenced_task_runner.h"
 #include "gin/object_template_builder.h"
 #include "gin/public/isolate_holder.h"
 #include "shell/common/gin_helper/dictionary.h"
@@ -90,7 +91,22 @@ void WrappableBase::SecondWeakCallback(
   if (gin::IsolateHolder::DestroyedMicrotasksRunner()) {
     return;
   }
-  delete static_cast<WrappableBase*>(data.GetInternalField(0));
+  // Defer destruction to a posted task. V8's second-pass weak callbacks run
+  // inside a DisallowJavascriptExecutionScope (they may touch the V8 API but
+  // must not invoke JS). Several Electron Wrappables (e.g. WebContents) emit
+  // JS events from their destructors, so deleting synchronously here can
+  // crash with "Invoke in DisallowJavascriptExecutionScope" — see
+  // https://github.com/electron/electron/issues/47420. Posting via the
+  // current sequence's task runner ensures the destructor runs once V8 has
+  // left the GC scope. If no task runner is available (e.g. early/late in
+  // process lifetime), fall back to synchronous deletion.
+  auto* wrappable = static_cast<WrappableBase*>(data.GetInternalField(0));
+  if (base::SequencedTaskRunner::HasCurrentDefault()) {
+    base::SequencedTaskRunner::GetCurrentDefault()->DeleteSoon(FROM_HERE,
+                                                               wrappable);
+  } else {
+    delete wrappable;
+  }
 }
 
 DeprecatedWrappableBase::DeprecatedWrappableBase() = default;
@@ -126,9 +142,19 @@ void DeprecatedWrappableBase::SecondWeakCallback(
     const v8::WeakCallbackInfo<DeprecatedWrappableBase>& data) {
   if (gin::IsolateHolder::DestroyedMicrotasksRunner())
     return;
+  // See WrappableBase::SecondWeakCallback for why deletion is posted: V8's
+  // second-pass weak callbacks run inside a DisallowJavascriptExecutionScope,
+  // and several Wrappables emit JS events from their destructors.
+  // https://github.com/electron/electron/issues/47420
   DeprecatedWrappableBase* wrappable = data.GetParameter();
-  if (wrappable)
+  if (!wrappable)
+    return;
+  if (base::SequencedTaskRunner::HasCurrentDefault()) {
+    base::SequencedTaskRunner::GetCurrentDefault()->DeleteSoon(FROM_HERE,
+                                                               wrappable);
+  } else {
     delete wrappable;
+  }
 }
 
 v8::MaybeLocal<v8::Object> DeprecatedWrappableBase::GetWrapperImpl(

shell/common/gin_helper/wrappable_base.h

@@ -6,6 +6,7 @@
 #define ELECTRON_SHELL_COMMON_GIN_HELPER_WRAPPABLE_BASE_H_
 
 #include "base/memory/raw_ptr.h"
+#include "base/task/sequenced_task_runner_helpers.h"
 #include "v8/include/v8-forward.h"
 
 namespace gin {
@@ -75,6 +76,11 @@ class DeprecatedWrappableBase {
   DeprecatedWrappableBase();
   virtual ~DeprecatedWrappableBase();
 
+  // SecondWeakCallback posts destruction via DeleteSoon so that destructors
+  // (which may emit JS events) run outside V8's GC scope. DeleteSoon needs
+  // access to the protected destructor.
+  friend class base::DeleteHelper<DeprecatedWrappableBase>;
+
   // Overrides of this method should be declared final and not overridden again.
   virtual gin::ObjectTemplateBuilder GetObjectTemplateBuilder(
       v8::Isolate* isolate);

spec/fixtures/crash-cases/wrappable-gc-weak-callback-emit/index.js

@@ -0,0 +1,29 @@
+const { app, WebContentsView } = require('electron');
+
+const v8 = require('node:v8');
+
+// Force V8 to schedule incremental-marking finalization steps as foreground
+// tasks on every allocation. Those tasks run inside V8's
+// DisallowJavascriptExecutionScope. Prior to the fix in
+// shell/common/gin_helper/wrappable.cc, gin_helper::SecondWeakCallback would
+// `delete` the Wrappable synchronously inside that scope, and Wrappables
+// whose destructors emit JS events (WebContents emits 'will-destroy' /
+// 'destroyed') would crash with "Invoke in DisallowJavascriptExecutionScope".
+//
+// Regression test for https://github.com/electron/electron/issues/47420.
+v8.setFlagsFromString('--stress-incremental-marking');
+
+app.whenReady().then(() => {
+  // Leak several WebContentsView instances so at least one wrapper's
+  // collection lands in a foreground GC task during app.quit()'s uv_run
+  // drain. Three is enough for the crash to reproduce 10/10 on a Linux
+  // testing build before the fix.
+  // eslint-disable-next-line no-new
+  new WebContentsView();
+  // eslint-disable-next-line no-new
+  new WebContentsView();
+  // eslint-disable-next-line no-new
+  new WebContentsView();
+
+  app.quit();
+});