amadan
ios: fix IntegrityMonitor background crash (concurrent UserDefaults writes) (#1194)
* ios: keep ReadState UserDefaults writes on the main actor
ThreadStore.fold() runs on a detached background thread and wrote the
server read-watermark straight to UserDefaults.standard there. That's a
process-global store shared with other writers (notably IntegrityMonitor's
save() on the main actor during a history fold), and UserDefaults is NOT
safe against concurrent writes from two threads: the background write races
the main-thread one and corrupts CFPrefs' internal CFBasicHash mid-encode,
crashing in CFBasicHashGetPtrIndex / __CFBinaryPlistWriteOrPresize.
The fold already carries the raw watermark back to the main actor in
Folded.readMarks, so move the forward-only ReadState.setLastSeen write into
refresh()'s main-actor readMarks loop and drop it from the detached fold.
Every UserDefaults writer in this file is now on the main actor. The old
in-fold comment claimed the write was "thread-safe here" — it wasn't; the
comment now records why the write must stay on the main actor.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* ios: coalesce IntegrityMonitor saves during a history fold
foldPageDetached calls noteGossip -> IntegrityMonitor.gossipIn once per
decrypted message and reaction in the page, and gossipIn saved on every
call — so loading one page re-encoded the whole IntegrityState blob and
rewrote UserDefaults dozens of times in a tight main-thread loop. That both
wastes work and widens the window for the cross-thread CFPrefs race the
sibling ReadState fix closes.
Add withCoalescedSave { } — a re-entrant scope that defers the disk write to
a single flush at scope exit — and wrap the page-fold gossip loop in it.
Alert publishing (recomputeUnresolved / onAlert) still fires per call, so the
failure banner reacts immediately; only the UserDefaults write is batched.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
3 files changed,
+59
−13
ios/LChatCore/Sources/LChatCore/ChatFeedModel.swift+7 −3ios/LChatCore/Sources/LChatCore/IntegrityMonitor.swift+36 −2ios/LChatCore/Sources/LChatCore/ThreadStore.swift+16 −8
diff --git a/ios/LChatCore/Sources/LChatCore/ChatFeedModel.swift b/ios/LChatCore/Sources/LChatCore/ChatFeedModel.swift| index cebd902..b268918 100644 |
| --- a/ios/LChatCore/Sources/LChatCore/ChatFeedModel.swift |
| +++ b/ios/LChatCore/Sources/LChatCore/ChatFeedModel.swift |
| @@ -1015,9 +1015,13 @@ public final class ChatFeedModel: ObservableObject { |
| // Tamper-evidence (history): compare gossip from every message + reaction |
| // this page decrypted — the server can route forged messages via |
| // catch-up, so history must count too, not just live frames. |
| - if integrity != nil { |
| - for s in stored { noteGossip(s.payload, own: s.payload.uid == myUserId) } |
| - for p in reactionRows { noteGossip(p, own: p.uid == myUserId) } |
| + if let integrity { |
| + // One coalesced save for the whole page — gossipIn otherwise writes |
| + // UserDefaults once per message (see IntegrityMonitor.withCoalescedSave). |
| + integrity.withCoalescedSave { |
| + for s in stored { noteGossip(s.payload, own: s.payload.uid == myUserId) } |
| + for p in reactionRows { noteGossip(p, own: p.uid == myUserId) } |
| + } |
| } |
| if !appEvents.isEmpty { |
| for (id, e) in appEvents { groupAppsEvents[id] = e } |
diff --git a/ios/LChatCore/Sources/LChatCore/IntegrityMonitor.swift b/ios/LChatCore/Sources/LChatCore/IntegrityMonitor.swift| index b14e14e..4639dfa 100644 |
| --- a/ios/LChatCore/Sources/LChatCore/IntegrityMonitor.swift |
| +++ b/ios/LChatCore/Sources/LChatCore/IntegrityMonitor.swift |
| @@ -2,8 +2,15 @@ |
| // twin of integrity.js's createIntegrity. Polls GET /api/checkpoints, verifies |
| // each new checkpoint, remembers the verified head, and compares gossiped peer |
| // heads to detect forks/rollback/withholding. @MainActor + single-process, so |
| -// (unlike the web's multi-tab localStorage) there is no cross-writer race to |
| -// guard — the feed and the poll timer are the same actor. |
| +// (unlike the web's multi-tab localStorage) the monitor never races ITSELF — |
| +// the feed and the poll timer are the same actor. It does NOT follow that its |
| +// UserDefaults persistence is race-free: UserDefaults.standard is a |
| +// process-global store shared with other writers (e.g. ThreadStore's read |
| +// watermark), so every writer of it must stay on the main actor or a |
| +// background write will corrupt CFPrefs mid-encode. Two rules keep that true |
| +// here: this class is @MainActor (so save() runs on main), and folding a whole |
| +// page of history coalesces into ONE save via withCoalescedSave (below) rather |
| +// than a per-message burst of main-thread writes. |
| // |
| // Detection only in this slice: alerts are recorded and published; how they |
| // surface (the failure banner, /integrity) is the UI layer. `onAlert` fires |
| @@ -70,6 +77,12 @@ public final class IntegrityMonitor: ObservableObject { |
| private var state: IntegrityState |
| + // Coalesced-save scope (see withCoalescedSave): while deferring, save() |
| + // just flags the state dirty and returns; the flush happens once when the |
| + // outermost scope exits. |
| + private var saveDeferred = false |
| + private var saveDirty = false |
| + |
| public init(store: IntegrityStore, now: @escaping () -> Int64 = { Int64(Date().timeIntervalSince1970 * 1000) }) { |
| self.store = store |
| self.now = now |
| @@ -109,6 +122,24 @@ public final class IntegrityMonitor: ObservableObject { |
| return v |
| } |
| + /// Run `body` with the per-call UserDefaults writes coalesced into a single |
| + /// save at the end. Folding a page of history calls gossipIn once per |
| + /// decrypted message; without this each call would re-encode and rewrite the |
| + /// whole state blob to UserDefaults on the main thread — a burst that both |
| + /// wastes work and widens the window for a cross-thread CFPrefs race. Alert |
| + /// publishing (recomputeUnresolved / onAlert) still fires per call, so the |
| + /// banner reacts immediately; only the disk write is deferred. Re-entrant. |
| + public func withCoalescedSave(_ body: () -> Void) { |
| + let wasDeferred = saveDeferred |
| + saveDeferred = true |
| + body() |
| + saveDeferred = wasDeferred |
| + if !saveDeferred && saveDirty { |
| + saveDirty = false |
| + save() |
| + } |
| + } |
| + |
| /// recordAlert lets other subsystems (e.g. key-transparency bind-MAC |
| /// verification) file an integrity alert through the same deduped, |
| /// banner-raising path the chain checks use. `key` distinguishes instances |
| @@ -322,6 +353,9 @@ public final class IntegrityMonitor: ObservableObject { |
| private func save() { |
| recomputeUnresolved() |
| + // Inside a withCoalescedSave scope, defer the actual disk write to the |
| + // single flush at scope exit (see withCoalescedSave). |
| + if saveDeferred { saveDirty = true; return } |
| if let data = try? JSONEncoder().encode(state) { store.saveIntegrity(data) } |
| } |
diff --git a/ios/LChatCore/Sources/LChatCore/ThreadStore.swift b/ios/LChatCore/Sources/LChatCore/ThreadStore.swift| index 5dee10e..d33d7de 100644 |
| --- a/ios/LChatCore/Sources/LChatCore/ThreadStore.swift |
| +++ b/ios/LChatCore/Sources/LChatCore/ThreadStore.swift |
| @@ -231,6 +231,12 @@ public final class ThreadStore: ObservableObject { |
| } |
| for (tid, mark) in folded.readMarks { |
| serverReadMarks[tid] = max(serverReadMarks[tid] ?? 0, mark) |
| + // Forward-only fold into the device-local watermark. Moved here from |
| + // the detached fold() so every UserDefaults write stays on the main |
| + // actor (see fold()); mark == 0 means the server had no watermark. |
| + if mark > ReadState.lastSeen(instance: instance.id, thread: tid) { |
| + ReadState.setLastSeen(instance: instance.id, thread: tid, at: mark) |
| + } |
| } |
| // A zero-thread fetch against rows we already hold is almost certainly |
| // a failed/offline fetch (the API calls degrade to empty) — keep what |
| @@ -487,15 +493,17 @@ public final class ThreadStore: ObservableObject { |
| let lastAt = t.last_at ?? 0 |
| // Fold the server's read watermark (advanced by ANY of this user's |
| // devices) into the local one — reading a chat on the phone clears |
| - // the dot here too. Forward-only, like the server: a stale local |
| - // value advances, a newer one (this device read something the |
| - // server hasn't confirmed yet) stands. (ReadState is UserDefaults — |
| - // thread-safe here; serverReadMarks is main-actor state, so the raw |
| - // value rides back in Folded.readMarks instead.) |
| + // the dot here too. The raw value rides back in Folded.readMarks; |
| + // the actual ReadState (UserDefaults) write — forward-only, like the |
| + // server: a stale local value advances, a newer one this device set |
| + // stands — happens back on the MAIN ACTOR in refresh(), never here. |
| + // fold() runs on a detached thread, and UserDefaults is NOT safe |
| + // against concurrent writes from another thread: a background write |
| + // racing a main-thread one (e.g. IntegrityMonitor's save) corrupts |
| + // CFPrefs' internal hash mid-encode → CFBasicHash crash. Keep every |
| + // UserDefaults write on the main actor. (serverReadMarks is likewise |
| + // main-actor state.) |
| readMarks[t.id] = t.last_read_at ?? 0 |
| - if let sm = t.last_read_at, sm > ReadState.lastSeen(instance: instance.id, thread: t.id) { |
| - ReadState.setLastSeen(instance: instance.id, thread: t.id, at: sm) |
| - } |
| // Park the key + decrypted title for the notification extension: a |
| // push's ciphertext is decrypted on-device against this store — |
| // plus the NICKNAMED members' names, so banners speak MY names for |