From 70f18e9a1c8bb999695e34819445b75318fe36e0 Mon Sep 17 00:00:00 2001
From: David Heinemeier Hansson <david@hey.com>
Date: Wed, 22 Jul 2026 21:39:15 -0700
Subject: [PATCH] Keep notification QObjects out of the ListModels to stop
 shell crashes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Model rows stored the live Notification object in a ref role. When the
server destroyed the notification (sender close, DND untrack, dismiss)
the role became a dangling C++ pointer, and the next read segfaulted in
QQmlListModel::data — typically when replaying history over IPC. Track
live notifications in a JS map keyed by originalId instead, cleaned up
on close and untrack, where a stale reference degrades to a catchable
error instead of a crash.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 .../notifications/NotificationLogic.js        |  6 +--
 shell/plugins/notifications/Service.qml       | 38 +++++++++++++++----
 2 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/shell/plugins/notifications/NotificationLogic.js b/shell/plugins/notifications/NotificationLogic.js
index 6f764036..09833e5d 100644
--- a/shell/plugins/notifications/NotificationLogic.js
+++ b/shell/plugins/notifications/NotificationLogic.js
@@ -73,8 +73,7 @@ function snapshotOf(notification, timestamp) {
     glyph: glyphFromHints(n.hints),
     urgency: n.urgency,
     expireTimeout: expireTimeout,
-    timestamp: timestamp === undefined ? Date.now() : timestamp,
-    ref: notification
+    timestamp: timestamp === undefined ? Date.now() : timestamp
   }
 }
 
@@ -91,8 +90,7 @@ function historyEntry(value, normalUrgency) {
     glyph: e.glyph || "",
     urgency: typeof e.urgency === "number" ? e.urgency : normalUrgency,
     expireTimeout: 0,
-    timestamp: e.timestamp || 0,
-    ref: null
+    timestamp: e.timestamp || 0
   }
 }
 
diff --git a/shell/plugins/notifications/Service.qml b/shell/plugins/notifications/Service.qml
index 3300a9ef..a2f0fcea 100644
--- a/shell/plugins/notifications/Service.qml
+++ b/shell/plugins/notifications/Service.qml
@@ -43,6 +43,13 @@ Item {
   readonly property int liveBarSize: shell && shell.bar && !shell.bar.barHidden ? Math.max(0, shell.bar.barSize) : defaultBarSize
   readonly property int barClearance: liveBarSize + Style.gapsOut
 
+  // Live Notification objects by originalId, kept OUT of the ListModels: a
+  // QObject stored in a model role becomes a dangling C++ pointer when the
+  // server destroys the notification (sender close, DND untrack, dismiss),
+  // and the next read of that role segfaults in QQmlListModel::data. A JS
+  // map only holds a wrapper, which degrades to a catchable error instead.
+  property var liveRefs: ({})
+
   // PersistentProperties handles in-process QML reloads. The on-disk
   // notifications.json file is the cross-restart backstop — its `dnd` key
   // is hydrated into persisted.doNotDisturb on startup and written back via
@@ -136,6 +143,13 @@ Item {
     // captured for the popup card.
     notification.tracked = true
     var snapshot = snapshotOf(notification)
+    liveRefs[snapshot.originalId] = notification
+    // Guard the delete: a newer notification may have reused this originalId
+    // (freedesktop replaces_id) and taken over the map slot.
+    notification.closed.connect(function() {
+      if (service.liveRefs[snapshot.originalId] === notification)
+        delete service.liveRefs[snapshot.originalId]
+    })
     // History is for notifications from real apps (Slack, Discord, mailer,
     // etc.) — things the user might want to look back at. Skip the pending
     // / past bookkeeping when:
@@ -154,6 +168,7 @@ Item {
     var ephemeralApp = NotificationLogic.isEphemeralApp(appName)
     if (transient || ephemeralApp) {
       if (service.doNotDisturb && !shouldBypassDnd(notification)) {
+        delete liveRefs[snapshot.originalId]
         notification.tracked = false
         return
       }
@@ -180,6 +195,7 @@ Item {
     // force visibility, so critical alone isn't enough — we also require
     // the sender to be CLI-style. See shouldBypassDnd().
     if (service.doNotDisturb && !shouldBypassDnd(notification)) {
+      delete liveRefs[snapshot.originalId]
       notification.tracked = false
       return
     }
@@ -278,8 +294,8 @@ Item {
   function removePopup(index, reason) {
     if (index < 0 || index >= popupModel.count) return
     var entry = popupModel.get(index)
-    var ref = entry ? entry.ref : null
     var originalId = entry ? entry.originalId : -1
+    var ref = originalId >= 0 ? liveRefs[originalId] : null
     popupModel.remove(index)
     if (ref) {
       try {
@@ -365,16 +381,22 @@ Item {
   function invokePopupDefault(index) {
     if (index < 0 || index >= popupModel.count) return
     var entry = popupModel.get(index)
-    var ref = entry ? entry.ref : null
+    var ref = entry ? liveRefs[entry.originalId] : null
     var invoked = false
-    if (ref && ref.actions) {
-      for (var i = 0; i < ref.actions.length; i++) {
-        var action = ref.actions[i]
-        if (action && action.identifier === "default") {
-          try { action.invoke(); invoked = true } catch (e) { console.warn("invoke default failed:", e) }
-          break
+    try {
+      if (ref && ref.actions) {
+        for (var i = 0; i < ref.actions.length; i++) {
+          var action = ref.actions[i]
+          if (action && action.identifier === "default") {
+            action.invoke()
+            invoked = true
+            break
+          }
         }
       }
+    } catch (e) {
+      // Notification already torn down by the server — fall through to focus.
+      console.warn("invoke default failed:", e)
     }
     // Chat apps (Slack, Discord, Vesktop, etc.) rarely register a "default"
     // libnotify action — they just expect clicking the notification to