Repository: electron/electron (language: cpp)

Issue:
WebFrameMain frames and name field access SIGSEGV
### Preflight Checklist

- [x] I have read the [Contributing Guidelines](https://github.com/electron/electron/blob/main/CONTRIBUTING.md) for this project.
- [x] I agree to follow the [Code of Conduct](https://github.com/electron/electron/blob/main/CODE_OF_CONDUCT.md) that this project adheres to.
- [x] I have searched the [issue tracker](https://www.github.com/electron/electron/issues) for a bug report that matches the one I want to file, without success.

### Electron Version

33.3.0

### What operating system(s) are you using?

macOS

### Operating System Version

macOS Sonoma 14.3

### What arch are you using?

arm64 (including Apple Silicon)

### Last Known Working Electron version

_No response_

### Expected Behavior

Accessing WebFrameMain `frames` and `name` attributes on destroyed frames previously threw an error.

### Actual Behavior

Accessing WebFrameMain `frames` and `name` attributes on destroyed frames now segfaults.

```checked if frame is alive 0.8441429125447275
checking if frame is alive 0.7526827941185381
checked if frame is destroyed 0.7526827941185381
checking if frame is detached 0.7526827941185381
checked if frame is detached 0.7526827941185381
checking .frames 0.7526827941185381
.../app/node_modules/electron/dist/Electron.app/Contents/MacOS/Electron exited with signal SIGSEGV```

The trace:

```
Thread 0 (crashed)
0 Electron Framework!content::FrameTree::SubtreeAndInnerTreeNodes(content::RenderFrameHostImpl*, bool) [frame_tree.cc : 353 + 0x0]
x0 = 0xefefefefefefefef x1 = 0x0000000000000000
x2 = 0x000000010c47eb00 x3 = 0x0000000000000000
x4 = 0x0000000000000000 x5 = 0x000000000000000f
x6 = 0x0000000000000180 x7 = 0x0000000000000004
x8 = 0xaaaaaaaaaaaaaaaa x9 = 0x000000010eb2cd94
x10 = 0xffffffffffff1e48  x11 = 0x00000d6300100000
x12 = 0x0000000000001685  x13 = 0x0000000000000044
x14 = 0x0000000000000010  x15 = 0x0000000000000003
x16 = 0x000000000089cb29  x17 = 0x0000000157e0f6cc
x18 = 0x0000000000000000  x19 = 0x000000016d59b710
x20 = 0x0000000000000000  x21 = 0x00000104009d7e00
x22 = 0x0000000000000000  x23 = 0x000000011528a000
x24 = 0x00000001153b8000  x25 = 0xefefefefefefefef
x26 = 0x0000010400510100  x27 = 0x00000d63002cdf65
x28 = 0x00000d6300000000 fp = 0x000000016d59b680
lr = 0x000000010eaf767c sp = 0x000000016d59b600
pc = 0x000000010ea57460
Found by: given as instruction pointer in context
1 Electron Framework!content::RenderFrameHostImpl::ForEachRenderFrameHostImpl(base::FunctionRef&lt;content::RenderFrameHost::FrameIterationAction (content::RenderFrameHostImpl*)&gt;, bool) [render_frame_host_impl.cc : 2713 + 0xc]
x19 = 0x0000000000000000  x20 = 0x000000010eb2cd9c
x21 = 0x000000016d59b790  x22 = 0x00000104009d7e00
x23 = 0xaaaaaaaaaaaaaaaa  x24 = 0x00000001153b8000
x25 = 0x00000104000f0280  x26 = 0x0000010400510100
x27 = 0x00000d63002cdf65  x28 = 0x00000d6300000000
fp = 0x000000016d59b770 sp = 0x000000016d59b690
pc = 0x000000010eaf767c
Found by: call frame info
2 Electron Framework!content::RenderFrameHostImpl::ForEachRenderFrameHost(base::FunctionRef&lt;void (content::RenderFrameHost*)&gt;) [render_frame_host_impl.cc : 2638 + 0x8]
x19 = 0x00000104067ccfa0  x20 = 0x000000016d59b7e8
x21 = 0x000000016d59b8a8  x22 = 0x0000000000000001
x23 = 0x0000000000000015  x24 = 0x00000001153b8000
x25 = 0x00000104000f0280  x26 = 0x0000010400510100
x27 = 0x00000d63002cdf65  x28 = 0x00000d6300000000
fp = 0x000000016d59b7a0 sp = 0x000000016d59b780
pc = 0x000000010eaf75c0
Found by: call frame info
3 Electron Framework!electron::api::WebFrameMain::Frames() const [electron_api_web_frame_main.cc : 412 + 0x4]
x19 = 0x00000104067ccfa0  x20 = 0x000000016d59b7e8
x21 = 0x000000016d59b8a8  x22 = 0x0000000000000001
x23 = 0x0000000000000015  x24 = 0x00000001153b8000
x25 = 0x00000104000f0280  x26 = 0x0000010400510100
x27 = 0x00000d63002cdf65  x28 = 0x00000d6300000000
fp = 0x000000016d59b7d0 sp = 0x000000016d59b7b0
pc = 0x000000010c47bf8c
Found by: call frame info```

### Testcase Gist URL

_No response_

### Additional Information

This may be related to [#43473](https://github.com/electron/electron/pull/43473) @samuelmaddock 


Discussion / hints:
<!-- blocked/need-repro -->

Hello @alexi. Thanks for reporting this and helping to make Electron better!

Would it be possible for you to make a standalone testcase with only the code necessary to reproduce the issue? For example, [Electron Fiddle](https://www.electronjs.org/fiddle) is a great tool for making small test cases and makes it easy to publish your test case to a [gist](https://gist.github.com) that Electron maintainers can use.

Stand-alone test cases make fixing issues go more smoothly: it ensure everyone's looking at the same issue, it removes all unnecessary variables from the equation, and it can also provide the basis for automated regression tests.

Now adding the https://github.com/electron/electron/labels/blocked%2Fneed-repro label for this reason. After you make a test case, please link to it in a followup comment. This issue will be closed in 10 days if the above is not addressed.
Reproduction: https://gist.github.com/alexi/8533563332a7c7004d0d535259080c18
The clue region is commented in the gist:

``` 
  // If we comment this handler out, the SIGSEGV does not occur.
  view.webContents.on("frame-created", (event, details) => {
      const { frame } = details;

      if (!frame) return;

      const { parent } = frame;
      if (!parent) return;

      // If we return here the crash _does_ still occur.
      // return; 

      frame.once("dom-ready", () => fixPreloadScripts(frame));
  });
```

However, in our production project the same does not hold true. Removing the on `frame-created` handler does not prevent the segfault.
I think this may be due to the fact that [CheckRenderFrame](https://github.com/electron/electron/blob/2745771a22dc7aec4364fec758804e0b7f230357/shell/browser/api/electron_api_web_frame_main.cc#L186) does not check if `render_frame_` is null, only if `render_frame_disposed_` is true.

One simple solution may be to add a null pointer check.
Wanted to check in and see if there is a timeline for adressing this segfault, it’s currently blocking us. Thanks!
Thanks for reporting the crash!

I tried reproducing this issue using the provided testcase gist but it does not produce a crash for me. It also seems to me like the fiddle contains a lot of extra code that might not be necessary.

Would you be able to 1) double-check the gist and 2) slim it down to only the minimal amount of code necessary to reproduce the issue?

Apart from that, it would help if you could determine

1) If the crash also happens on v35 prereleases
2) What version of Electron this last worked on.

Thank you very much in advance!
Interesting!

I have slimmed down the gist: https://gist.github.com/alexi/e0fd317c0663267433c6c766e870ddbe

1. The crash does happen for me on v35.0.0-alpha.5
2. The last working version: v32.3.0
Thanks for slimming down the Fiddle gist!

For everyone running it: It's set to run on v32.3.0 by default — the version where it's supposed to work and not crash.

I'm still not able to reproduce the crash with the slimmed-down gist (on macOS 15.2 with Electron 34/35) but maybe someone else is.
I just reproduced on another machine as well (macOS Sonoma 14.5 - v33.3.0, v34.0.2).
I believe there is a one line change that would fix this, planning to get local dev env setup to test but it's:

```
WebFrameMain::CheckRenderFrame() {
    if (render_frame_disposed_ || render_frame_ == nullptr) { // add nullptr check
```
@alexi I'll test out this change and see if this fixes it. Thanks for doing so much research on this!

Fix patch:
diff --git a/shell/browser/api/electron_api_web_frame_main.cc b/shell/browser/api/electron_api_web_frame_main.cc
index 3e800659a48c3..cc3d142eef049 100644
--- a/shell/browser/api/electron_api_web_frame_main.cc
+++ b/shell/browser/api/electron_api_web_frame_main.cc
@@ -183,7 +183,7 @@ void WebFrameMain::UpdateRenderFrameHost(content::RenderFrameHost* rfh) {
 }
 
 bool WebFrameMain::CheckRenderFrame() const {
-  if (render_frame_disposed_) {
+  if (!HasRenderFrame()) {
     v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
     v8::HandleScope scope(isolate);
     gin_helper::ErrorThrower(isolate).ThrowError(
@@ -435,7 +435,7 @@ v8::Local<v8::Promise> WebFrameMain::CollectDocumentJSCallStack(
   gin_helper::Promise<base::Value> promise(args->isolate());
   v8::Local<v8::Promise> handle = promise.GetHandle();
 
-  if (render_frame_disposed_) {
+  if (!HasRenderFrame()) {
     promise.RejectWithErrorMessage(
         "Render frame was disposed before WebFrameMain could be accessed");
     return handle;
@@ -463,7 +463,7 @@ void WebFrameMain::CollectedJavaScriptCallStack(
     gin_helper::Promise<base::Value> promise,
     const std::string& untrusted_javascript_call_stack,
     const std::optional<blink::LocalFrameToken>& remote_frame_token) {
-  if (render_frame_disposed_) {
+  if (!HasRenderFrame()) {
     promise.RejectWithErrorMessage(
         "Render frame was disposed before call stack was received");
     return;
diff --git a/shell/browser/api/electron_api_web_frame_main.h b/shell/browser/api/electron_api_web_frame_main.h
index 9191e6f346217..de984aee5db69 100644
--- a/shell/browser/api/electron_api_web_frame_main.h
+++ b/shell/browser/api/electron_api_web_frame_main.h
@@ -101,8 +101,14 @@ class WebFrameMain final : public gin::Wrappable<WebFrameMain>,
   void TeardownMojoConnection();
   void OnRendererConnectionError();
 
-  // WebFrameMain can outlive its RenderFrameHost pointer so we need to check
-  // whether its been disposed of prior to accessing it.
+  [[nodiscard]] constexpr bool HasRenderFrame() const {
+    return !render_frame_disposed_ && render_frame_ != nullptr;
+  }
+
+  // Throws a JS error if HasRenderFrame() is false.
+  // WebFrameMain can outlive its RenderFrameHost pointer,
+  // so we need to check whether its been disposed of
+  // prior to accessing it.
   bool CheckRenderFrame() const;
 
   v8::Local<v8::Promise> ExecuteJavaScript(gin::Arguments* args,


Test patch: