Repository: electron/electron
Pull request: fix: defer Wrappable destruction in SecondWeakCallback to a posted task

## Description of Change

V8's second-pass weak callbacks run inside a `DisallowJavascriptExecutionScope` — they may touch the V8 API but **must not invoke JavaScript**, directly or indirectly. Several Electron `gin_helper::Wrappable` subclasses (`WebContents` in particular) emit JS events from their destructors (`'will-destroy'` / `'destroyed'`), so when `SecondWeakCallback` synchronously `delete`s the C++ object inside that scope, the destructor's `Emit()` → `node::MakeCallback` → `v8::Function::Call` aborts with:

```
# Fatal error in ../../v8/src/execution/execution.cc, line 364
# Invoke in DisallowJavascriptExecutionScope
```

This was previously latent and timing-dependent — it required GC to collect the JS wrapper from a foreground GC task posted by V8's incremental marking finalizer, which only happened under specific heap-layout / allocation-timing conditions. It surfaced as #47420, #45416, podman-desktop/podman-desktop#12409, and intermittent flakes of `spec/fixtures/crash-cases/webcontentsview-create-leak-exit`.

### Fix

Both `WrappableBase::SecondWeakCallback` and `DeprecatedWrappableBase::SecondWeakCallback` now post the deletion via `base::SequencedTaskRunner::GetCurrentDefault()->{DeleteSoon,PostTask}` so the destructor (and any `Emit()` it does) runs once V8 has left the GC scope. Falls back to synchronous deletion when no task runner is bound (early/late process lifetime).

### Regression test

`spec/fixtures/crash-cases/wrappable-gc-weak-callback-emit/` reproduces the crash deterministically on `main` by setting `v8.setFlagsFromString('--stress-incremental-marking')` (which forces foreground GC tasks on every allocation step) and leaking three `WebContentsView` instances before `app.quit()`. Verified on a Linux testing build:

| | runs | crashes |
|---|---|---|
| Before fix | 5/5 | 5 |
| After fix | 10/10 | 0 |

All 23 existing crash-case fixtures continue to pass.

### Checklist

- [x] PR description has a clear explanation of the change
- [x] `npm run lint` passes
- [x] regression test added (`spec/fixtures/crash-cases/wrappable-gc-weak-callback-emit`)
- [x] all `spec/fixtures/crash-cases/*` pass

Fixes #47420
Fixes #45416

Notes: Fixed an intermittent `Invoke in DisallowJavascriptExecutionScope` crash on application quit when a `WebContents` (or other JS-emitting native object) is garbage-collected during shutdown.


Changed files:
filename: shell/common/gin_helper/wrappable.cc
patch: @@ -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(

filename: shell/common/gin_helper/wrappable_base.h
patch: @@ -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);

filename: spec/fixtures/crash-cases/wrappable-gc-weak-callback-emit/index.js
patch: @@ -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();
+});