Repository: electron/electron (language: cpp)

Issue:
[Bug]: Electron crashes with libnotify 0.8.3 in portal environment when closing notification
### 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.0.0, 27.0.3 and 28.0.0-beta.2

### What operating system are you using?

Other Linux

### Operating System Version

- Arch Linux
- `uname -a`: `Linux hostname 6.5.7-arch1-1 #1 SMP PREEMPT_DYNAMIC Tue, 10 Oct 2023 21:10:21 +0000 x86_64 GNU/Linux`

### What arch are you using?

x64

### Last Known Working Electron version

Not sure

### Expected Behavior

Electron does not crash when running `notification.close()`.

### Actual Behavior

Electron crashes with `notification.close()` when in portal (flatpak/snap, or `NOTIFY_FORCE_PORTAL=1` is set).

### Testcase Gist URL

_No response_

### Additional Information

An MRE modified from electron-quick-start: https://github.com/taoky/electron-libnotify-portal-crash-mre.

Screencast:

https://github.com/electron/electron/assets/2109893/9bc49b03-ddef-4599-93e6-e7b4fb168b29

My analysis:

1. `LibnotifyNotification::Show()` binds "closed" glib signal to `LibnotifyNotification::OnNotificationClosed` https://github.com/electron/electron/blob/7999ea39e2ee9702e64e4125e8e89c314e9f468f/shell/browser/notifications/linux/libnotify_notification.cc#L91-L94
2. When closing (dismissing) notification, `LibnotifyNotification::Dismiss()` is called which runs `libnotify_loader_.notify_notification_close()`: https://github.com/electron/electron/blob/7999ea39e2ee9702e64e4125e8e89c314e9f468f/shell/browser/notifications/linux/libnotify_notification.cc#L167
3. In `notify_notification_close()`, as libnotify 0.8.x adds supports for portal environment, it will call `remove_portal_notification()` instead of using gdbus call (<https://gitlab.gnome.org/GNOME/libnotify/-/blob/0.8.3/libnotify/notification.c#L1761>), then it calls `close_notification()` (<https://gitlab.gnome.org/GNOME/libnotify/-/blob/0.8.3/libnotify/notification.c#L825>), and nightmare happens: it `g_signal_emit (notification, signals[SIGNAL_CLOSED], 0);` (<https://gitlab.gnome.org/GNOME/libnotify/-/blob/0.8.3/libnotify/notification.c#L707>).
4. `LibnotifyNotification::OnNotificationClosed()` happily calls `NotificationDismissed()`, which finally calls `Destroy()`, and then `presenter()->RemoveNotification(this)`, and `this` is then `delete`d inside.
5. `Notification::Close()` in electron_api_notification.cc has no idea about `notification_` being deleted, it continues, and crashes:
https://github.com/electron/electron/blob/7999ea39e2ee9702e64e4125e8e89c314e9f468f/shell/browser/api/electron_api_notification.cc#L217C6-L227


Discussion / hints:
libnotify 0.8.x, **before 0.8.3** is affected by another bug: https://gitlab.gnome.org/GNOME/libnotify/-/issues/34.

But libnotify 0.8.3 has fixed its own bug, as I guess it's time for Electron to fix this unexpected `LibnotifyNotification::OnNotificationClosed()` call inside `LibnotifyNotification::Dismiss()`.

Workaround for app dev: Don't use `notification.close()` to avoid crash.
This issue has been automatically marked as stale. **If this issue is still affecting you, please leave any comment** (for example, "bump"), and we'll keep it open. If you have any new additional information—in particular, if this is still reproducible in the [latest version of Electron](https://www.electronjs.org/releases/stable) or in the [beta](https://www.electronjs.org/releases/beta)—please include it with your comment!
I'm sure this bug still exists in electron 28.2.1 and 29.0.0-beta.6.
just tested it in 28.2.3, there it is still an issue
You can also work around this crash by forcing libnotify to use dbus instead of the portal with the env var `NOTIFY_IGNORE_PORTAL=1`.

You need to add this to your flatpak build file:
```yaml
finish-args:
   - --talk-name=org.freedesktop.Notifications
   - --env=NOTIFY_IGNORE_PORTAL=1
```

example from my project: https://github.com/flathub/chat.delta.desktop/pull/125/commits/cf25cd0fa8c12a14cd52ee4c1600114b5b12f35e

Fix patch:
diff --git a/shell/browser/notifications/linux/libnotify_notification.cc b/shell/browser/notifications/linux/libnotify_notification.cc
index e039b382e3e24..4c8452fad1a8d 100644
--- a/shell/browser/notifications/linux/libnotify_notification.cc
+++ b/shell/browser/notifications/linux/libnotify_notification.cc
@@ -159,21 +159,21 @@ void LibnotifyNotification::Show(const NotificationOptions& options) {
 
 void LibnotifyNotification::Dismiss() {
   if (!notification_) {
-    Destroy();
     return;
   }
 
   GError* error = nullptr;
+  on_dismissing_ = true;
   libnotify_loader_.notify_notification_close(notification_, &error);
   if (error) {
     log_and_clear_error(error, "notify_notification_close");
-    Destroy();
   }
+  on_dismissing_ = false;
 }
 
 void LibnotifyNotification::OnNotificationClosed(
     NotifyNotification* notification) {
-  NotificationDismissed();
+  NotificationDismissed(!on_dismissing_);
 }
 
 void LibnotifyNotification::OnNotificationView(NotifyNotification* notification,
diff --git a/shell/browser/notifications/linux/libnotify_notification.h b/shell/browser/notifications/linux/libnotify_notification.h
index 8aacd0952062a..315419fc5c734 100644
--- a/shell/browser/notifications/linux/libnotify_notification.h
+++ b/shell/browser/notifications/linux/libnotify_notification.h
@@ -33,6 +33,7 @@ class LibnotifyNotification : public Notification {
   RAW_PTR_EXCLUSION NotifyNotification* notification_ = nullptr;
 
   ScopedGSignal signal_;
+  bool on_dismissing_ = false;
 };
 
 }  // namespace electron


Test patch: