Repository: electron/electron (language: cpp)

Issue:
[Bug]: Screen sharing crashes app on Ubuntu 22.04.2 LTS
### 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

25.3.0

### What operating system are you using?

Other Linux

### Operating System Version

Ubuntu 22.04.2 LTS

### What arch are you using?

x64

### Last Known Working Electron version

21.4.4

### Expected Behavior

App should not crash when using [desktopCapturer](https://www.electronjs.org/docs/latest/api/desktop-capturer) on Ubuntu 22.04.2 LTS (Gnome `42.9` / PipeWire `0.3.48` / Wayland).

### Actual Behavior

App crashes, see [video](https://www.youtube.com/watch?v=5gw6e7zEsKc).

```console
[10441:0717/143633.964276:ERROR:egl_dmabuf.cc(608)] Failed to record frame: Error creating EGLImage - EGL_NOT_INITIALIZED
[10441:0717/143633.964294:ERROR:shared_screencast_stream.cc(940)] Dropping DMA-BUF modifier: 72057594037927935 and trying to renegotiate stream parameters
```

To reproduce issue,

1. Install [Ubuntu 22.04.2 LTS](https://ubuntu.com/download/desktop) (issue is not present in live mode)
2. Install curl, git and libfuse2 (`sudo add-apt-repository -y universe && sudo apt install -y curl git libfuse2`)
3. Install Electron Fiddle (`curl -LO https://github.com/electron/fiddle/releases/download/v0.33.0/electron-fiddle_0.33.0_amd64.deb && sudo dpkg -i electron-fiddle_0.33.0_amd64.deb`)
4. Load and run `https://gist.github.com/c4e0a8bf08f4fea0680abe0702e93a1c` using Electron Fiddle
5. Click “Step 1” button and select window
6. Click “Step 2” button
7. Move app window

### Testcase Gist URL

https://gist.github.com/c4e0a8bf08f4fea0680abe0702e93a1c

### Additional Information

_No response_


Discussion / hints:
Electron Fiddle appears to be using Electron 25.2, not Electron 25.3 which contains the fix.
Hey @orowith2os, thanks for helping out!

I believe one has to enable and select version 25.3.0 using version menu… above [gist](https://gist.github.com/c4e0a8bf08f4fea0680abe0702e93a1c) and [video](https://www.youtube.com/watch?v=5gw6e7zEsKc) are using 25.3.0.
I can reproduce this on ArmCord 3.2.1, except instead of crashing, it logs the EGL and PipeWire renegotiations thousands of times a second, causing the memory usage to increase *drastically*. Then again, it crashes once in a while too, using the X11 backend. Wayland seems to be better.
The error messages might not mean anything. I do see them occasionally, but a failed renegotiation should not cause a crash. I tried to reproduce this on a VM with Ubuntu 22.04, but I could not. Can you try collecting a crash dump?

You can follow [these instructions](https://github.com/electron/electron/issues/37463#issuecomment-1459206621) to modify the fiddle. In Electron Fiddle's settings, enable "Do not delete user data directories" under the "Execution" section to retain the dmp file. You can either share the dmp file or symbolicate it using [electron-minidump](https://www.npmjs.com/package/electron-minidump) and share the logs.

Do you see this issue with Chromium? Are you using an Nvidia GPU? If you're not, can you check if you can reproduce this with `linux-generic-hwe-22.04-edge` kernel?
Thanks for helping out @aiddya.

> Can you try collecting a crash dump?

Strangely enough, I had a hard time getting app to crash when using `crashReporter.start`… without [revision](https://gist.github.com/sunknudsen/c4e0a8bf08f4fea0680abe0702e93a1c/revisions#diff-58417e0f781b6656949d37258c8b9052ed266e2eb7a5163cad7b0863e6b2916aR1-R7), app crashed pretty consistently.

Thankfully, app did crash… please see following `electron-minidump` file.

[359bdcd8-6d2d-4f01-b23d-1c9954fe0755.txt](https://github.com/electron/electron/files/12105224/359bdcd8-6d2d-4f01-b23d-1c9954fe0755.txt)

> Do you see this issue with Chromium?

No, tried to crash Chromium using Jitsi.

> Are you using an Nvidia GPU?

No, using Intel “Alder Lake-P Integrated Graphics Controller”.

> If you're not, can you check if you can reproduce this with linux-generic-hwe-22.04-edge kernel?

I don’t know how to switch kernel… hopefully above dump is enough. If it isn’t, happy to dig into switching kernel.
It looks like `electron-minidump` was not able to find the symbols for Electron and PipeWire. Can you share the original files from the crash directory?

Even with the obfuscated stack trace, I can see that the crash is in `libpipewire-0.3.so.0`. This library is provided by the distro and is dynamically opened by Electron. I suspect that the bug is in the version of `libpipewire` shipped by Ubuntu 22.04, which is pretty old at this point. A properly symbolicated crash log could confirm this and you could open a bug report on Ubuntu.
FWIW this is reproducible with the PipeWire in org.freedesktop.Platform 22.08.12.1, on ArmCord 3.2.1.

The shipped version is 0.3.69-0-gcd8be0ba3b27542253f7744b699c2ede159e2d7c. They're having issues updating the runtime right now, so it's not on 22.08.19, but I will test again when that's resolved.
Hey @aiddya,

> Can you share the original files from the crash directory?

Sure, is there anything sensitive in the dump? Is all you need the file that I fed to `electron-minidump`?

> I suspect that the bug is in the version of libpipewire shipped by Ubuntu 22.04, which is pretty old at this point.

Would you happen to know how one can update `libpipewire`?

> A properly symbolicated crash log could confirm this and you could open a bug report on Ubuntu.

This stuff is above my head but how would one explain why app doesn’t crash when using live mode?

No, the dump contains a bunch of information about the crashing process. Since the process in question is just test Electron application, it will not have anything sensitive.

The easiest method to upgrade PipeWire is upgrade your OS to the latest available version, that is Ubuntu 23.04. There are some alternative methods such as using PPAs, but I can't recommend them as they could possibly leave your system in an inconsistent state.

There could be many reasons for not seeing the crash in live mode. One is package versions—the live mode contains versions of packages from the time the ISO was created and not the latest ones. The second might be hardware acceleration, which may not be active in live mode. PipeWire tries to use shared GPU buffers (DMA-BUFs) when possible, but falls back to copying. The crash might be in the DMA-BUF code path and not the copying code path. Finally, the crash could be caused by a race condition and the live might be slow enough to not trigger the race condition.
Hey @aiddya, thanks so much for thorough explanation and for your work on issue. 🙌

Here is the dump… [359bdcd8-6d2d-4f01-b23d-1c9954fe0755.dmp.zip](https://github.com/electron/electron/files/12131379/359bdcd8-6d2d-4f01-b23d-1c9954fe0755.dmp.zip)

Here are the crash logs with electron symbols: [crash_logs.txt](https://github.com/electron/electron/files/12136915/crash_logs.txt)

Unfortunately, it looks like Ubuntu has not generated debug symbol packages for the newest version of PipeWire packages. I can see that the crash was in a [call to `pw_stream_update_params` during renegotiation](https://webrtc.googlesource.com/src/+/d20849d0710783142175551d81dfe0dcbffcf2b1/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc#389). However, it's hard to investigate further, unless the crash can be reproduced in an OS that does generate debug symbols for all packages.

Looking at recent WebRTC commits, there's [one commit that looks relevant](https://webrtc.googlesource.com/src/+/8fcc6df79daf1810cd4ecdb8d2ef1d361abfdc9c%5E%21/). It's a buffer overflow issue that could cause a crash, although the [bug report](https://bugs.chromium.org/p/webrtc/issues/detail?id=15346) says that PipeWire notices and guards against the overflow. Perhaps that's not the case in older versions of PipeWire.

@sunknudsen Can you try to reproduce the issue on `27.0.0-nightly.20230717` and `27.0.0-nightly.20230714`? You'll have to enable "Beta" and "Nightly" channels in the "Electron" section of Fiddle's settings to see those versions in the drop down. 
Hey @aiddya,

> Can you try to reproduce the issue on 27.0.0-nightly.20230717 and 27.0.0-nightly.20230714?

For sure… tried using `27.0.0-nightly.20230724` and app didn’t crash even though “Dropping DMA-BUF modifier” error was encountered.

```console
[8030:0725/062033.728772:ERROR:gl_surface_presentation_helper.cc(260)] GetVSyncParametersIfAvailable() failed for 1 times!
[8030:0725/062037.663999:ERROR:gl_surface_presentation_helper.cc(260)] GetVSyncParametersIfAvailable() failed for 2 times!
[7993:0725/062101.637575:ERROR:egl_dmabuf.cc(608)] Failed to record frame: Error creating EGLImage - EGL_NOT_INITIALIZED
[7993:0725/062101.637592:ERROR:shared_screencast_stream.cc(940)] Dropping DMA-BUF modifier: 72057594037927935 and trying to renegotiate stream parameters
[8030:0725/062228.824620:ERROR:gl_surface_presentation_helper.cc(260)] GetVSyncParametersIfAvailable() failed for 3 times!

Electron exited with code 0.
```

Not sure what has changed, but Electron appears to have become more stable which is amazing.

Also, new combined screen or window picker greatly improves UX. 🙌

Will improvements make it to `v25.3.2` release?

That's encouraging, but can you test with both of the exact versions from my comment—`27.0.0-nightly.20230717` and `27.0.0-nightly.20230714`. It's necessary to isolate the commit that fixed the issue.
Hey @aiddya, wasn’t able to consistently crash app on `27.0.0-nightly.20230714` but crashes did occur… wasn’t able to crash app a single time on `27.0.0-nightly.20230717`.


Great, thanks! That confirms that [311541: PipeWire capturer: increase buffer size to avoid buffer overflow](https://webrtc-review.googlesource.com/c/src/+/311541) will fix the issue. This is already on main, but it'll have to be backported to stable branches. While we're at it, it might be worth picking up [305120: PipeWire capturer: fix fcntl call when duplicating a file descriptor](https://webrtc-review.googlesource.com/c/src/+/305120) as well, as it fixes another PipeWire issue.

Fix patch:
diff --git a/patches/webrtc/.patches b/patches/webrtc/.patches
index 883cd06a6f023..b4e2887b502dc 100644
--- a/patches/webrtc/.patches
+++ b/patches/webrtc/.patches
@@ -1,3 +1,5 @@
 fix_fallback_to_x11_capturer_on_wayland.patch
 m114_move_transceiver_iteration_loop_over_to_the_signaling_thread.patch
 m114_sdp_reject_duplicate_ssrcs_in_ssrc-groups.patch
+pipewire_capturer_fix_fcntl_call_when_duplicating_a_file_descriptor.patch
+pipewire_capturer_increase_buffer_size_to_avoid_buffer_overflow.patch
diff --git a/patches/webrtc/pipewire_capturer_fix_fcntl_call_when_duplicating_a_file_descriptor.patch b/patches/webrtc/pipewire_capturer_fix_fcntl_call_when_duplicating_a_file_descriptor.patch
new file mode 100644
index 0000000000000..4adca4ca9eb29
--- /dev/null
+++ b/patches/webrtc/pipewire_capturer_fix_fcntl_call_when_duplicating_a_file_descriptor.patch
@@ -0,0 +1,29 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Jan Grulich <grulja@gmail.com>
+Date: Fri, 12 May 2023 20:59:06 +0200
+Subject: PipeWire capturer: fix fcntl call when duplicating a file descriptor
+
+The fcntl() call has variable arguments, therefore we need to pass 0 to
+specify there are no other arguments for this call, otherwise we might
+end up with an argument that is random garbage.
+
+Bug: webrtc:15174
+Change-Id: I34f16a942d80913b667d8ade7eed557b0233be01
+Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/305120
+Reviewed-by: Alexander Cooper <alcooper@chromium.org>
+Commit-Queue: Jan Grulich <grulja@gmail.com>
+Cr-Commit-Position: refs/heads/main@{#40060}
+
+diff --git a/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc b/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc
+index d31f86772e348b7b6c103289180bdca3bf182a94..0aab2dd22466bc6b6d4e8e43fab8eb716512df5b 100644
+--- a/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc
++++ b/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc
+@@ -447,7 +447,7 @@ bool SharedScreenCastStreamPrivate::StartScreenCastStream(
+ 
+     if (fd >= 0) {
+       pw_core_ = pw_context_connect_fd(
+-          pw_context_, fcntl(fd, F_DUPFD_CLOEXEC), nullptr, 0);
++          pw_context_, fcntl(fd, F_DUPFD_CLOEXEC, 0), nullptr, 0);
+     } else {
+       pw_core_ = pw_context_connect(pw_context_, nullptr, 0);
+     }
diff --git a/patches/webrtc/pipewire_capturer_increase_buffer_size_to_avoid_buffer_overflow.patch b/patches/webrtc/pipewire_capturer_increase_buffer_size_to_avoid_buffer_overflow.patch
new file mode 100644
index 0000000000000..cb30685528455
--- /dev/null
+++ b/patches/webrtc/pipewire_capturer_increase_buffer_size_to_avoid_buffer_overflow.patch
@@ -0,0 +1,47 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Jan Grulich <grulja@gmail.com>
+Date: Mon, 10 Jul 2023 10:07:38 +0200
+Subject: PipeWire capturer: increase buffer size to avoid buffer overflow
+
+Recently added framerate option can cause a buffer overflow and make
+PipeWire to fail on negotiation, which effectively makes screen sharing
+not to work.
+
+Bug: webrtc:15346
+Change-Id: I4a68e26c8f85ca287b06a25da500b6a7009e075f
+Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/311541
+Reviewed-by: Alexander Cooper <alcooper@chromium.org>
+Commit-Queue: Jan Grulich <grulja@gmail.com>
+Cr-Commit-Position: refs/heads/main@{#40413}
+
+diff --git a/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc b/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc
+index 0aab2dd22466bc6b6d4e8e43fab8eb716512df5b..5b1cd10e9d903bcebf0389ec5c985fbc7ddff6a8 100644
+--- a/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc
++++ b/modules/desktop_capture/linux/wayland/shared_screencast_stream.cc
+@@ -276,7 +276,7 @@ void SharedScreenCastStreamPrivate::OnStreamParamChanged(
+ 
+   that->stream_size_ = DesktopSize(width, height);
+ 
+-  uint8_t buffer[1024] = {};
++  uint8_t buffer[2048] = {};
+   auto builder = spa_pod_builder{buffer, sizeof(buffer)};
+ 
+   // Setup buffers and meta header for new format.
+@@ -361,7 +361,7 @@ void SharedScreenCastStreamPrivate::OnRenegotiateFormat(void* data, uint64_t) {
+   {
+     PipeWireThreadLoopLock thread_loop_lock(that->pw_main_loop_);
+ 
+-    uint8_t buffer[2048] = {};
++    uint8_t buffer[4096] = {};
+ 
+     spa_pod_builder builder = spa_pod_builder{buffer, sizeof(buffer)};
+ 
+@@ -479,7 +479,7 @@ bool SharedScreenCastStreamPrivate::StartScreenCastStream(
+ 
+     pw_stream_add_listener(pw_stream_, &spa_stream_listener_,
+                            &pw_stream_events_, this);
+-    uint8_t buffer[2048] = {};
++    uint8_t buffer[4096] = {};
+ 
+     spa_pod_builder builder = spa_pod_builder{buffer, sizeof(buffer)};
+ 


Test patch: