Lyrics.persist() mutates metadata.data — a plain Swift Dictionary — and is reachable from two threads at once. Nothing synchronises them.
Call sites
On develop today:
AppController.currentTrackChanged (AppController.swift:329) — runs on DispatchQueue.lyricsDisplay.AppDelegate.applicationWillTerminate (AppDelegate.swift:158) — main thread.AppDelegate.showCurrentLyricsInFinder (AppDelegate.swift:272) — main thread.
showCurrentLyricsInFinder has exactly the shape that races: check needsPersist, call persist(), then read metadata.localURL. A track change landing on the lyrics-display queue at the same moment writes metadata.data[.localURL] and [.needsPersist] concurrently. Unsynchronised dictionary mutation can corrupt the storage or trap.
There is also a second effect: two interleaved Data.write(to:options:.atomic) calls against the same path.
Not a regression
This is not new, and it is not something #187 introduced — that PR adds a third instance of the same pattern (editCurrentLyrics), which is why it surfaced during review. git log shows no previous fix attempt for it, so it has been this way for as long as persist() has been callable from menu actions.
It is hard to trigger deliberately: it needs a track change to land inside the window between the needsPersist check and the write. That is why it should be fixed structurally rather than by chasing a reproduction.
Fix direction
Fixing the three call sites one at a time would leave the same trap for the next feature that touches currentLyrics from a menu action. The durable fix is to make the lyrics-display confinement compiler-checked — a @globalActor for that domain — so an off-queue mutation cannot compile. That is the design question open on #183; whichever way it goes, this needs to be closed out explicitly rather than assumed fixed.
Related: #183, #187.
Lyrics.persist()mutatesmetadata.data— a plain SwiftDictionary— and is reachable from two threads at once. Nothing synchronises them.Call sites
On
developtoday:AppController.currentTrackChanged(AppController.swift:329) — runs onDispatchQueue.lyricsDisplay.AppDelegate.applicationWillTerminate(AppDelegate.swift:158) — main thread.AppDelegate.showCurrentLyricsInFinder(AppDelegate.swift:272) — main thread.showCurrentLyricsInFinderhas exactly the shape that races: checkneedsPersist, callpersist(), then readmetadata.localURL. A track change landing on the lyrics-display queue at the same moment writesmetadata.data[.localURL]and[.needsPersist]concurrently. Unsynchronised dictionary mutation can corrupt the storage or trap.There is also a second effect: two interleaved
Data.write(to:options:.atomic)calls against the same path.Not a regression
This is not new, and it is not something #187 introduced — that PR adds a third instance of the same pattern (
editCurrentLyrics), which is why it surfaced during review.git logshows no previous fix attempt for it, so it has been this way for as long aspersist()has been callable from menu actions.It is hard to trigger deliberately: it needs a track change to land inside the window between the
needsPersistcheck and the write. That is why it should be fixed structurally rather than by chasing a reproduction.Fix direction
Fixing the three call sites one at a time would leave the same trap for the next feature that touches
currentLyricsfrom a menu action. The durable fix is to make the lyrics-display confinement compiler-checked — a@globalActorfor that domain — so an off-queue mutation cannot compile. That is the design question open on #183; whichever way it goes, this needs to be closed out explicitly rather than assumed fixed.Related: #183, #187.