Keep notification QObjects out of the ListModels to stop shell crashes

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>
This commit is contained in:
David Heinemeier Hansson
2026-07-22 21:39:15 -07:00
co-authored by Claude Fable 5
parent fc6168af86
commit 70f18e9a1c
2 changed files with 32 additions and 12 deletions
@@ -73,8 +73,7 @@ function snapshotOf(notification, timestamp) {
glyph: glyphFromHints(n.hints), glyph: glyphFromHints(n.hints),
urgency: n.urgency, urgency: n.urgency,
expireTimeout: expireTimeout, expireTimeout: expireTimeout,
timestamp: timestamp === undefined ? Date.now() : timestamp, timestamp: timestamp === undefined ? Date.now() : timestamp
ref: notification
} }
} }
@@ -91,8 +90,7 @@ function historyEntry(value, normalUrgency) {
glyph: e.glyph || "", glyph: e.glyph || "",
urgency: typeof e.urgency === "number" ? e.urgency : normalUrgency, urgency: typeof e.urgency === "number" ? e.urgency : normalUrgency,
expireTimeout: 0, expireTimeout: 0,
timestamp: e.timestamp || 0, timestamp: e.timestamp || 0
ref: null
} }
} }
+30 -8
View File
@@ -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 liveBarSize: shell && shell.bar && !shell.bar.barHidden ? Math.max(0, shell.bar.barSize) : defaultBarSize
readonly property int barClearance: liveBarSize + Style.gapsOut 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 // PersistentProperties handles in-process QML reloads. The on-disk
// notifications.json file is the cross-restart backstop — its `dnd` key // notifications.json file is the cross-restart backstop — its `dnd` key
// is hydrated into persisted.doNotDisturb on startup and written back via // is hydrated into persisted.doNotDisturb on startup and written back via
@@ -136,6 +143,13 @@ Item {
// captured for the popup card. // captured for the popup card.
notification.tracked = true notification.tracked = true
var snapshot = snapshotOf(notification) 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, // History is for notifications from real apps (Slack, Discord, mailer,
// etc.) — things the user might want to look back at. Skip the pending // etc.) — things the user might want to look back at. Skip the pending
// / past bookkeeping when: // / past bookkeeping when:
@@ -154,6 +168,7 @@ Item {
var ephemeralApp = NotificationLogic.isEphemeralApp(appName) var ephemeralApp = NotificationLogic.isEphemeralApp(appName)
if (transient || ephemeralApp) { if (transient || ephemeralApp) {
if (service.doNotDisturb && !shouldBypassDnd(notification)) { if (service.doNotDisturb && !shouldBypassDnd(notification)) {
delete liveRefs[snapshot.originalId]
notification.tracked = false notification.tracked = false
return return
} }
@@ -180,6 +195,7 @@ Item {
// force visibility, so critical alone isn't enough — we also require // force visibility, so critical alone isn't enough — we also require
// the sender to be CLI-style. See shouldBypassDnd(). // the sender to be CLI-style. See shouldBypassDnd().
if (service.doNotDisturb && !shouldBypassDnd(notification)) { if (service.doNotDisturb && !shouldBypassDnd(notification)) {
delete liveRefs[snapshot.originalId]
notification.tracked = false notification.tracked = false
return return
} }
@@ -278,8 +294,8 @@ Item {
function removePopup(index, reason) { function removePopup(index, reason) {
if (index < 0 || index >= popupModel.count) return if (index < 0 || index >= popupModel.count) return
var entry = popupModel.get(index) var entry = popupModel.get(index)
var ref = entry ? entry.ref : null
var originalId = entry ? entry.originalId : -1 var originalId = entry ? entry.originalId : -1
var ref = originalId >= 0 ? liveRefs[originalId] : null
popupModel.remove(index) popupModel.remove(index)
if (ref) { if (ref) {
try { try {
@@ -365,16 +381,22 @@ Item {
function invokePopupDefault(index) { function invokePopupDefault(index) {
if (index < 0 || index >= popupModel.count) return if (index < 0 || index >= popupModel.count) return
var entry = popupModel.get(index) var entry = popupModel.get(index)
var ref = entry ? entry.ref : null var ref = entry ? liveRefs[entry.originalId] : null
var invoked = false var invoked = false
if (ref && ref.actions) { try {
for (var i = 0; i < ref.actions.length; i++) { if (ref && ref.actions) {
var action = ref.actions[i] for (var i = 0; i < ref.actions.length; i++) {
if (action && action.identifier === "default") { var action = ref.actions[i]
try { action.invoke(); invoked = true } catch (e) { console.warn("invoke default failed:", e) } if (action && action.identifier === "default") {
break 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" // Chat apps (Slack, Discord, Vesktop, etc.) rarely register a "default"
// libnotify action — they just expect clicking the notification to // libnotify action — they just expect clicking the notification to