Return immediately on HealthKit query timeout · f1821a3

Return immediately on HealthKit query timeout
Browse files

bogdan committed 14 hours ago

main

1 parent ef37ba6

commit f1821a3

Showing 2 changed files with 119 additions and 23 deletions

+17 -0

HealthProbe/Doc/04-project/Import-Optimization-Log.md

@@ -105,6 +105,23 @@ Action taken:
 - keep failed `earliest_sample` behavior as failed/timeout, because no safe zero
   inference is possible when HealthKit did not answer the existence query.
 
+Follow-up root cause review:
+- user correctly suspected commit `36d0904` as the regression source;
+- the monotonic timer itself was not the problem, but the same commit also
+  removed the shared boundary-query deadline (`dateDeadline`) and gave both
+  `earliest_sample` and `latest_sample` a full per-query timeout;
+- the same commit rewrote `withTimeout` using `withThrowingTaskGroup`; when the
+  timeout child fired, the task-group scope could still wait for the operation
+  child that was blocked inside a HealthKit query;
+- this explains both symptoms: empty types spending timeout-scale time in
+  boundary queries and `Import page 1` remaining visible past the configured page
+  timeout.
+
+Additional action:
+- replace the task-group timeout race with a continuation-based race guarded by
+  a lock. The timeout path now cancels the HealthKit operation and resumes the
+  caller immediately instead of waiting for a stuck child task to unwind.
+
 ### 2026-06-07 Overnight Initial Imports On Both Large Devices
 
 Source: two user-provided overnight first-import reports from large devices,


+102 -23

HealthProbe/Services/HealthKitService.swift

View

+                         private func withTimeout<T: Sendable>(seconds: TimeInterval, operation: @escaping @Sendable () async throws -> T) async throws -> T {
+                             guard seconds > 0, seconds.isFinite else { throw CancellationError() }
+                             let nanoseconds = UInt64((seconds * 1_000_000_000).rounded(.up))
+                    -        let operationTask = Task {
+                    -            try await operation()
+                    -        }
+                    -        let timeoutTask = Task<T, Error> {
+                    -            try await Task.sleep(nanoseconds: nanoseconds)
+                    -            operationTask.cancel()
+                    -            throw CancellationError()
+                    -        }
+                    -        defer {
+                    -            operationTask.cancel()
+                    -            timeoutTask.cancel()
+                    -        }
+                    -        return try await withThrowingTaskGroup(of: T.self) { group in
+                    -            group.addTask { try await operationTask.value }
+                    -            group.addTask { try await timeoutTask.value }
+                    -            do {
+                    -                let result = try await group.next()!
+                    -                group.cancelAll()
+                    -                return result
+                    -            } catch {
+                    -                group.cancelAll()
+                    -                throw error
+                    +        let box = TimeoutContinuationBox<T>()
+                    +        return try await withTaskCancellationHandler {
+                    +            try await withCheckedThrowingContinuation { continuation in
+                    +                let operationTask = Task {
+                    +                    do {
+                    +                        let value = try await operation()
+                    +                        box.resume(continuation, with: .success(value))
+                    +                    } catch {
+                    +                        box.resume(continuation, with: .failure(error))
+                    +                    }
+                    +                }
+                    +                box.setOperationTask(operationTask)
+                    +                let timeoutTask = Task {
+                    +                    do {
+                    +                        try await Task.sleep(nanoseconds: nanoseconds)
+                    +                    } catch {
+                    +                        return
+                    +                    }
+                    +                    box.cancelOperation()
+                    +                    box.resume(continuation, with: .failure(CancellationError()))
+                    +                }
+                    +                box.setTimeoutTask(timeoutTask)
+                    +        } onCancel: {
+                    +            box.cancelAll()
+                         let recordArchiveData: Data?
+                    +private final class TimeoutContinuationBox<Value: Sendable>: @unchecked Sendable {
+                    +    private let lock = NSLock()
+                    +    nonisolated(unsafe) private var didResume = false
+                    +    nonisolated(unsafe) private var operationTask: Task<Void, Never>?
+                    +    nonisolated(unsafe) private var timeoutTask: Task<Void, Never>?
+                    +    nonisolated func setOperationTask(_ task: Task<Void, Never>) {
+                    +        setTask(task, keyPath: \.operationTask)
+                    +    }
+                    +    nonisolated func setTimeoutTask(_ task: Task<Void, Never>) {
+                    +        setTask(task, keyPath: \.timeoutTask)
+                    +    }
+                    +    nonisolated func cancelOperation() {
+                    +        lock.lock()
+                    +        let task = operationTask
+                    +        lock.unlock()
+                    +        task?.cancel()
+                    +    }
+                    +    nonisolated func cancelAll() {
+                    +        lock.lock()
+                    +        let operationTask = operationTask
+                    +        let timeoutTask = timeoutTask
+                    +        lock.unlock()
+                    +        operationTask?.cancel()
+                    +        timeoutTask?.cancel()
+                    +    }
+                    +    nonisolated func resume(
+                    +        _ continuation: CheckedContinuation<Value, Error>,
+                    +        with result: Result<Value, Error>
+                    +    ) {
+                    +        let shouldResume: Bool
+                    +        let operationTask: Task<Void, Never>?
+                    +        let timeoutTask: Task<Void, Never>?
+                    +        lock.lock()
+                    +        if didResume {
+                    +            shouldResume = false
+                    +            operationTask = nil
+                    +            timeoutTask = nil
+                    +        } else {
+                    +            didResume = true
+                    +            shouldResume = true
+                    +            operationTask = self.operationTask
+                    +            timeoutTask = self.timeoutTask
+                    +        }
+                    +        lock.unlock()
+                    +        guard shouldResume else { return }
+                    +        operationTask?.cancel()
+                    +        timeoutTask?.cancel()
+                    +        continuation.resume(with: result)
+                    +    }
+                    +    private nonisolated func setTask(
+                    +        _ task: Task<Void, Never>,
+                    +        keyPath: ReferenceWritableKeyPath<TimeoutContinuationBox<Value>, Task<Void, Never>?>
+                    +    ) {
+                    +        let shouldCancel: Bool
+                    +        lock.lock()
+                    +        if didResume {
+                    +            shouldCancel = true
+                    +        } else {
+                    +            shouldCancel = false
+                    +            self[keyPath: keyPath] = task
+                    +        }
+                    +        lock.unlock()
+                    +        if shouldCancel {
+                    +            task.cancel()
+                    +        }
+                    +    }
+                    +}
+                     private final class HealthKitQueryContinuationBox<Value: Sendable>: @unchecked Sendable {
+                         private let lock = NSLock()
+                         nonisolated(unsafe) private var continuation: CheckedContinuation<Value, Error>?

	@@ -105,6 +105,23 @@ Action taken:
105	105	- keep failed `earliest_sample` behavior as failed/timeout, because no safe zero
106	106	inference is possible when HealthKit did not answer the existence query.
107	107
	108	+Follow-up root cause review:
	109	+- user correctly suspected commit `36d0904` as the regression source;
	110	+- the monotonic timer itself was not the problem, but the same commit also
	111	+ removed the shared boundary-query deadline (`dateDeadline`) and gave both
	112	+ `earliest_sample` and `latest_sample` a full per-query timeout;
	113	+- the same commit rewrote `withTimeout` using `withThrowingTaskGroup`; when the
	114	+ timeout child fired, the task-group scope could still wait for the operation
	115	+ child that was blocked inside a HealthKit query;
	116	+- this explains both symptoms: empty types spending timeout-scale time in
	117	+ boundary queries and `Import page 1` remaining visible past the configured page
	118	+ timeout.
	119	+
	120	+Additional action:
	121	+- replace the task-group timeout race with a continuation-based race guarded by
	122	+ a lock. The timeout path now cancels the HealthKit operation and resumes the
	123	+ caller immediately instead of waiting for a stuck child task to unwind.
	124	+
108	125	### 2026-06-07 Overnight Initial Imports On Both Large Devices
109	126
110	127	Source: two user-provided overnight first-import reports from large devices,