Skip to content

Commit 7d8fffc

Browse files
committed
terminate helper reliably
1 parent b948786 commit 7d8fffc

3 files changed

Lines changed: 125 additions & 6 deletions

File tree

Sources/ClaudeCost/AppDelegate.swift

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, NSMenuDelegate {
77

88
private var statusItem: NSStatusItem?
99
private var timer: Timer?
10+
private var refreshTask: Task<Void, Never>?
1011
private var state = AppState()
1112
private let loginItemManager = LoginItemManager()
1213
private var startAtLoginViewState = StartAtLoginViewState.make(status: .notRegistered)
@@ -35,6 +36,9 @@ final class AppDelegate: NSObject, NSApplicationDelegate, NSMenuDelegate {
3536
func applicationWillTerminate(_ notification: Notification) {
3637
timer?.invalidate()
3738
timer = nil
39+
refreshTask?.cancel()
40+
refreshTask = nil
41+
UsageFetcher.cancelActiveHelper()
3842
}
3943

4044
func menuNeedsUpdate(_ menu: NSMenu) {
@@ -71,13 +75,19 @@ final class AppDelegate: NSObject, NSApplicationDelegate, NSMenuDelegate {
7175
state = nextState
7276
renderTitle()
7377

74-
Task {
78+
refreshTask?.cancel()
79+
refreshTask = Task {
7580
do {
7681
let snapshot = try await UsageFetcher.fetchUsage()
7782
applyRefreshSuccess(snapshot)
7883
} catch {
84+
guard !Task.isCancelled else {
85+
return
86+
}
7987
applyRefreshFailure(error)
8088
}
89+
90+
refreshTask = nil
8191
}
8292
}
8393

Sources/ClaudeCost/UsageFetcher.swift

Lines changed: 102 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import Foundation
33
private final class ProcessCompletionState: @unchecked Sendable {
44
private let lock = NSLock()
55
private var hasResumed = false
6+
private var timedOut = false
67

78
func tryMarkResumed() -> Bool {
89
lock.lock()
@@ -15,6 +16,46 @@ private final class ProcessCompletionState: @unchecked Sendable {
1516
hasResumed = true
1617
return true
1718
}
19+
20+
func markTimedOut() {
21+
lock.lock()
22+
defer { lock.unlock() }
23+
timedOut = true
24+
}
25+
26+
func didTimeOut() -> Bool {
27+
lock.lock()
28+
defer { lock.unlock() }
29+
return timedOut
30+
}
31+
}
32+
33+
private final class HelperProcessRegistry: @unchecked Sendable {
34+
private let lock = NSLock()
35+
private var process: Process?
36+
37+
func set(_ process: Process) {
38+
lock.lock()
39+
defer { lock.unlock() }
40+
self.process = process
41+
}
42+
43+
func clear(_ process: Process) {
44+
lock.lock()
45+
defer { lock.unlock() }
46+
47+
guard self.process === process else {
48+
return
49+
}
50+
51+
self.process = nil
52+
}
53+
54+
func currentProcess() -> Process? {
55+
lock.lock()
56+
defer { lock.unlock() }
57+
return process
58+
}
1859
}
1960

2061
struct UsagePayload: Codable {
@@ -63,19 +104,38 @@ enum UsageFetcherError: LocalizedError {
63104

64105
enum UsageFetcher {
65106
private static let helperTimeout: TimeInterval = 30
107+
private static let helperTerminationGracePeriod: TimeInterval = 2
108+
private static let helperTerminationPollInterval: UInt32 = 50_000
109+
private static let helperRegistry = HelperProcessRegistry()
66110

67111
static func fetchUsage() async throws -> UsageSnapshot {
68-
let monthStart = currentMonthStartString()
69-
let helperURL = try usageHelperURL()
112+
try await withTaskCancellationHandler {
113+
let monthStart = currentMonthStartString()
114+
let helperURL = try usageHelperURL()
70115

71-
let output = try await runHelper(at: helperURL, arguments: [monthStart])
72-
return try UsagePayloadParser.decodeSnapshot(from: output)
116+
let output = try await runHelper(at: helperURL, arguments: [monthStart])
117+
return try UsagePayloadParser.decodeSnapshot(from: output)
118+
} onCancel: {
119+
cancelActiveHelper()
120+
}
73121
}
74122

75123
static func shouldTimeOut(processIsRunning: Bool) -> Bool {
76124
processIsRunning
77125
}
78126

127+
static func shouldEscalateTermination(processIsRunning: Bool, waitedEnough: Bool) -> Bool {
128+
processIsRunning && waitedEnough
129+
}
130+
131+
static func cancelActiveHelper() {
132+
guard let process = helperRegistry.currentProcess() else {
133+
return
134+
}
135+
136+
terminateAndReap(process)
137+
}
138+
79139
private static func usageHelperURL() throws -> URL {
80140
if let override = ProcessInfo.processInfo.environment["CLAUDECOST_HELPER_PATH"],
81141
!override.isEmpty
@@ -121,12 +181,19 @@ enum UsageFetcher {
121181
process.standardError = stderr
122182

123183
process.terminationHandler = { process in
184+
helperRegistry.clear(process)
185+
124186
let stdoutData = stdout.fileHandleForReading.readDataToEndOfFile()
125187
let stderrData = stderr.fileHandleForReading.readDataToEndOfFile()
126188
let stderrString =
127189
String(data: stderrData, encoding: .utf8)?
128190
.trimmingCharacters(in: .whitespacesAndNewlines) ?? ""
129191

192+
if completionState.didTimeOut() {
193+
resumeOnce(with: .failure(UsageFetcherError.timedOut(helperURL.path)))
194+
return
195+
}
196+
130197
guard process.terminationStatus == 0 else {
131198
resumeOnce(
132199
with: .failure(UsageFetcherError.commandFailed(helperURL.path, stderrString))
@@ -144,6 +211,7 @@ enum UsageFetcher {
144211

145212
do {
146213
try process.run()
214+
helperRegistry.set(process)
147215
} catch {
148216
resumeOnce(with: .failure(UsageFetcherError.failedToStart))
149217
return
@@ -154,12 +222,41 @@ enum UsageFetcher {
154222
return
155223
}
156224

157-
process.terminate()
225+
completionState.markTimedOut()
226+
terminateAndReap(process)
158227
resumeOnce(with: .failure(UsageFetcherError.timedOut(helperURL.path)))
159228
}
160229
}
161230
}
162231

232+
private static func terminateAndReap(_ process: Process) {
233+
guard process.processIdentifier > 0 else {
234+
return
235+
}
236+
237+
if process.isRunning {
238+
process.terminate()
239+
}
240+
241+
waitForExit(of: process, timeout: helperTerminationGracePeriod)
242+
243+
if shouldEscalateTermination(
244+
processIsRunning: process.isRunning,
245+
waitedEnough: true
246+
) {
247+
kill(process.processIdentifier, SIGKILL)
248+
waitForExit(of: process, timeout: helperTerminationGracePeriod)
249+
}
250+
}
251+
252+
private static func waitForExit(of process: Process, timeout: TimeInterval) {
253+
let deadline = Date().addingTimeInterval(timeout)
254+
255+
while process.isRunning && Date() < deadline {
256+
usleep(helperTerminationPollInterval)
257+
}
258+
}
259+
163260
private static func currentMonthStartString(now: Date = Date()) -> String {
164261
let formatter = DateFormatter()
165262
formatter.calendar = Calendar.current

Tests/UsageFetcherHarness.swift

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,16 @@ func testUsageFetcherTimeoutDecision() throws {
99
!UsageFetcher.shouldTimeOut(processIsRunning: false),
1010
"completed helper should not trigger timeout handling"
1111
)
12+
try expect(
13+
UsageFetcher.shouldEscalateTermination(processIsRunning: true, waitedEnough: true),
14+
"running helper should escalate after the grace period"
15+
)
16+
try expect(
17+
!UsageFetcher.shouldEscalateTermination(processIsRunning: true, waitedEnough: false),
18+
"helper should not escalate before the grace period expires"
19+
)
20+
try expect(
21+
!UsageFetcher.shouldEscalateTermination(processIsRunning: false, waitedEnough: true),
22+
"stopped helper should not escalate"
23+
)
1224
}

0 commit comments

Comments
 (0)