Skip to content

Commit 31f4ef4

Browse files
committed
Backport PR jordanbaird#923 spacing fixes (ac/sourcetree)
Apply ac/sourcetree's three fixes to MenuBarItemSpacingManager verbatim from the upstream patch: 1. applyOffset's per-PID guard uses continue instead of break, so hitting Control Center / the Ice process / a stale NSRunningApplication no longer silently aborts the whole apply. Each skip also logs the reason at debug level. 2. signalAppToQuit's continuation is guarded with a tryClaimOnce()-style OSAllocatedUnfairLock<Bool> helper so the force-terminate failsafe and the KVO observer can't both call resume and trap. A 1-second failsafe inside the timeout task makes sure we resume even if KVO never fires post-terminate. 3. launchApp takes a replacedPID and excludes it from the 'already running' check, so auto-respawning helpers (Login Items, launchd-managed agents) that come back under a new PID are still recognized as fresh rather than matching the dead process.
1 parent 67bbd5e commit 31f4ef4

2 files changed

Lines changed: 53 additions & 11 deletions

File tree

Ice/MenuBar/Spacing/MenuBarItemSpacingManager.swift

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import Cocoa
77
import Combine
88
import OSLog
9+
import os.lock
910

1011
/// Manager for menu bar item spacing.
1112
@MainActor
@@ -84,6 +85,7 @@ final class MenuBarItemSpacingManager {
8485
app.terminate()
8586

8687
var cancellable: AnyCancellable?
88+
let didResume = OSAllocatedUnfairLock(initialState: false)
8789
return try await withCheckedThrowingContinuation { continuation in
8890
let timeoutTask = Task {
8991
try await Task.sleep(for: .seconds(forceTerminateDelay))
@@ -95,6 +97,12 @@ final class MenuBarItemSpacingManager {
9597
"""
9698
)
9799
app.forceTerminate()
100+
// Failsafe: if KVO doesn't fire after force terminate, resume anyway.
101+
try? await Task.sleep(for: .seconds(1))
102+
cancellable?.cancel()
103+
if didResume.tryClaimOnce() {
104+
continuation.resume()
105+
}
98106
}
99107
}
100108

@@ -108,15 +116,25 @@ final class MenuBarItemSpacingManager {
108116
timeoutTask.cancel()
109117
cancellable?.cancel()
110118
logger.debug("Application \"\(app.logString, privacy: .public)\" terminated successfully")
111-
continuation.resume()
119+
if didResume.tryClaimOnce() {
120+
continuation.resume()
121+
}
112122
}
113123
}
114124
}
115125

116126
/// Asynchronously launches the app at the given URL.
117-
private nonisolated func launchApp(at applicationURL: URL, bundleIdentifier: String) async throws {
118-
if let app = NSWorkspace.shared.runningApplications.first(where: { $0.bundleIdentifier == bundleIdentifier }) {
119-
logger.debug("Application \"\(app.logString, privacy: .public)\" is already open, so skipping launch")
127+
///
128+
/// `replacedPID` is the PID of the just-terminated process. The
129+
/// "already running" check ignores any process matching that PID,
130+
/// so an auto-respawn under a new PID is recognized as fresh,
131+
/// while an unexpected match against the dead PID falls through to
132+
/// `openApplication(at:)`.
133+
private nonisolated func launchApp(at applicationURL: URL, bundleIdentifier: String, replacedPID: pid_t) async throws {
134+
if let app = NSWorkspace.shared.runningApplications.first(where: {
135+
$0.bundleIdentifier == bundleIdentifier && $0.processIdentifier != replacedPID
136+
}) {
137+
logger.debug("Application \"\(app.logString, privacy: .public)\" already running at PID \(app.processIdentifier, privacy: .public), skipping launch")
120138
return
121139
}
122140
let configuration = NSWorkspace.OpenConfiguration()
@@ -136,9 +154,10 @@ final class MenuBarItemSpacingManager {
136154
else {
137155
throw RelaunchError()
138156
}
157+
let oldPID = app.processIdentifier
139158
try await signalAppToQuit(app)
140159
if app.isTerminated {
141-
try await launchApp(at: url, bundleIdentifier: bundleIdentifier)
160+
try await launchApp(at: url, bundleIdentifier: bundleIdentifier, replacedPID: oldPID)
142161
} else {
143162
throw RelaunchError()
144163
}
@@ -160,22 +179,30 @@ final class MenuBarItemSpacingManager {
160179

161180
let items = await MenuBarItem.getMenuBarItems(option: .activeSpace)
162181
let pids = Set(items.map { $0.sourcePID ?? $0.ownerPID })
182+
logger.debug("applyOffset: \(items.count, privacy: .public) items, \(pids.count, privacy: .public) unique PIDs: \(pids.map { String($0) }.sorted().joined(separator: ", "), privacy: .public)")
163183

164184
var failedApps = [String]()
165185

166186
await withTaskGroup(of: Void.self) { group in
167187
for pid in pids {
168-
guard
169-
let app = NSRunningApplication(processIdentifier: pid),
170-
app.bundleIdentifier != "com.apple.controlcenter", // ControlCenter handles its own relaunch, so skip it.
171-
app != .current
172-
else {
173-
break
188+
guard let app = NSRunningApplication(processIdentifier: pid) else {
189+
logger.debug("applyOffset: skipping pid \(pid, privacy: .public) (no NSRunningApplication)")
190+
continue
191+
}
192+
guard app.bundleIdentifier != "com.apple.controlcenter" else {
193+
logger.debug("applyOffset: skipping pid \(pid, privacy: .public) (Control Center)")
194+
continue
195+
}
196+
guard app != .current else {
197+
logger.debug("applyOffset: skipping pid \(pid, privacy: .public) (self)")
198+
continue
174199
}
200+
logger.debug("applyOffset: relaunching pid \(pid, privacy: .public) bundle=\(app.bundleIdentifier ?? "<nil>", privacy: .public)")
175201
group.addTask { @MainActor in
176202
do {
177203
try await self.relaunchApp(app)
178204
} catch {
205+
self.logger.debug("applyOffset: relaunch FAILED for pid \(app.processIdentifier, privacy: .public) bundle=\(app.bundleIdentifier ?? "<nil>", privacy: .public) error=\(String(describing: error), privacy: .public)")
179206
guard let name = app.localizedName else {
180207
return
181208
}

Ice/Utilities/ConcurrencyHelpers.swift

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,3 +110,18 @@ extension Task where Failure == any Error {
110110
}
111111
}
112112
}
113+
114+
// MARK: - OSAllocatedUnfairLock
115+
116+
extension OSAllocatedUnfairLock where State == Bool {
117+
/// Atomically sets the value to `true` and returns whether this call
118+
/// was the first to do so. Useful for ensuring a continuation or
119+
/// callback is invoked exactly once across competing code paths.
120+
func tryClaimOnce() -> Bool {
121+
withLock { claimed in
122+
let wasUnclaimed = !claimed
123+
claimed = true
124+
return wasUnclaimed
125+
}
126+
}
127+
}

0 commit comments

Comments
 (0)