diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e4d723c5..6887ff3f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,6 +42,9 @@ jobs: - name: Validate privacy configuration run: python3 -m unittest tests.test_privacy_configuration + - name: Validate timer lifecycle cleanup + run: python3 -m unittest tests.test_timer_lifecycle + - name: Download Metal Toolchain # The downloadable Metal Toolchain is an Xcode 26+ (macOS 26) component; older Xcode ships metal. run: | diff --git a/.gitignore b/.gitignore index 632bf871..17167ffe 100644 --- a/.gitignore +++ b/.gitignore @@ -140,6 +140,7 @@ PrivateWorks *.py !tests/test_privacy_configuration.py +!tests/test_timer_lifecycle.py *.sh comparison_report.json *.txt diff --git a/DynamicIsland/managers/SystemTimerBridge.swift b/DynamicIsland/managers/SystemTimerBridge.swift index a5e34685..5ca2e36b 100644 --- a/DynamicIsland/managers/SystemTimerBridge.swift +++ b/DynamicIsland/managers/SystemTimerBridge.swift @@ -822,12 +822,17 @@ final class SystemTimerBridge { DispatchQueue.main.async { guard TimerManager.shared.isExternalTimerActive else { return } if didComplete { + guard let completedSessionID = TimerManager.shared.completedSessionID else { return } self.logDebug("Timer finished; scheduling delayed cleanup") DispatchQueue.main.asyncAfter(deadline: .now() + 3.2) { - if TimerManager.shared.isExternalTimerActive { - self.logDebug("Delayed cleanup firing; ending external timer") - TimerManager.shared.endExternalTimer(triggerSmoothClose: false) + guard TimerManager.shared.activeSource == .external, + TimerManager.shared.activeSessionID == completedSessionID else { + self.logDebug("Skipping delayed cleanup because a newer timer session is active") + return } + + self.logDebug("Delayed cleanup firing; ending external timer") + TimerManager.shared.endExternalTimer(triggerSmoothClose: false) } } else { self.logDebug("Ending external timer immediately") diff --git a/DynamicIsland/managers/TimerLifecycle.swift b/DynamicIsland/managers/TimerLifecycle.swift new file mode 100644 index 00000000..88a4835c --- /dev/null +++ b/DynamicIsland/managers/TimerLifecycle.swift @@ -0,0 +1,41 @@ +/* + * Atoll (DynamicIsland) + * Copyright (C) 2024-2026 Atoll Contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + */ + +import Foundation + +/// Gives each timer run its own identity so delayed work from an earlier run +/// cannot mutate a timer that was started in the meantime. +struct TimerLifecycle { + private(set) var sessionID: UUID? + private(set) var completedSessionID: UUID? + + @discardableResult + mutating func beginSession() -> UUID { + let sessionID = UUID() + self.sessionID = sessionID + completedSessionID = nil + return sessionID + } + + @discardableResult + mutating func completeSession() -> UUID? { + completedSessionID = sessionID + return completedSessionID + } + + mutating func endSession() { + sessionID = nil + completedSessionID = nil + } + + func isCurrent(_ candidate: UUID) -> Bool { + sessionID == candidate + } +} diff --git a/DynamicIsland/managers/TimerManager.swift b/DynamicIsland/managers/TimerManager.swift index 88fe3876..8a7d9222 100644 --- a/DynamicIsland/managers/TimerManager.swift +++ b/DynamicIsland/managers/TimerManager.swift @@ -125,6 +125,20 @@ class TimerManager: ObservableObject { private var timerInstance: Timer? private var cancellables = Set() private var soundPlayer: AVAudioPlayer? + private var smoothCloseWorkItem: DispatchWorkItem? + private var lifecycle = TimerLifecycle() + + /// Identifies the current timer run. Delayed cleanup must match this value + /// before changing presentation state. + var activeSessionID: UUID? { + lifecycle.sessionID + } + + /// Session that most recently reached completion. Starting a replacement + /// session clears this value before any delayed cleanup can capture it. + var completedSessionID: UUID? { + lifecycle.completedSessionID + } // MARK: - Initialization private init() { // Simple initialization @@ -132,6 +146,7 @@ class TimerManager: ObservableObject { deinit { timerInstance?.invalidate() + smoothCloseWorkItem?.cancel() soundPlayer?.stop() cancellables.removeAll() } @@ -144,6 +159,8 @@ class TimerManager: ObservableObject { // Stop any existing timer timerInstance?.invalidate() + soundPlayer?.stop() + beginTimerSession() // Start new timer withAnimation(.smooth) { @@ -275,6 +292,7 @@ class TimerManager: ObservableObject { timerInstance?.invalidate() timerInstance = nil soundPlayer?.stop() + beginTimerSession() activeSource = .external @@ -298,6 +316,16 @@ class TimerManager: ObservableObject { func updateExternalTimer(remaining: TimeInterval, totalDuration: TimeInterval?, isPaused paused: Bool, name: String? = nil) { guard activeSource == .external else { return } + // Clock may reuse both the timer ID and duration. A positive update + // after completion is nevertheless a new run and must invalidate the + // previous run's delayed close. + if remaining > 0 && (isFinished || isOvertime || remainingTime <= 0) { + beginTimerSession() + withAnimation(.smooth) { + isTimerActive = true + } + } + if let name, !name.isEmpty, name != timerName { timerName = name } @@ -319,6 +347,7 @@ class TimerManager: ObservableObject { func completeExternalTimer() { guard activeSource == .external else { return } + lifecycle.completeSession() remainingTime = 0 elapsedTime = totalDuration isOvertime = false @@ -342,6 +371,7 @@ class TimerManager: ObservableObject { } private func resetTimer() { + endTimerSession() withAnimation(.smooth) { isTimerActive = false } @@ -381,12 +411,33 @@ class TimerManager: ObservableObject { } private func scheduleSmoothClose() { + guard let sessionID = activeSessionID else { return } + + smoothCloseWorkItem?.cancel() + // Wait 3 seconds then smoothly close the live activity - DispatchQueue.main.asyncAfter(deadline: .now() + 3.0) { + let workItem = DispatchWorkItem { [weak self] in + guard let self, self.lifecycle.isCurrent(sessionID) else { return } + self.smoothCloseWorkItem = nil withAnimation(.easeInOut(duration: 1.0)) { self.isTimerActive = false } } + + smoothCloseWorkItem = workItem + DispatchQueue.main.asyncAfter(deadline: .now() + 3.0, execute: workItem) + } + + private func beginTimerSession() { + smoothCloseWorkItem?.cancel() + smoothCloseWorkItem = nil + lifecycle.beginSession() + } + + private func endTimerSession() { + smoothCloseWorkItem?.cancel() + smoothCloseWorkItem = nil + lifecycle.endSession() } private func playTimerSound() { diff --git a/tests/TimerLifecycleRegression.swift b/tests/TimerLifecycleRegression.swift new file mode 100644 index 00000000..03804a9a --- /dev/null +++ b/tests/TimerLifecycleRegression.swift @@ -0,0 +1,32 @@ +import Foundation + +@main +struct TimerLifecycleRegression { + static func main() { + var lifecycle = TimerLifecycle() + + let completedTimer = lifecycle.beginSession() + precondition(lifecycle.isCurrent(completedTimer)) + precondition(lifecycle.completeSession() == completedTimer) + precondition(lifecycle.completedSessionID == completedTimer) + var islandIsVisible = true + + // Reusing the same duration or external timer ID must still create a + // different run. Cleanup captured for completedTimer is now stale. + let replacementTimer = lifecycle.beginSession() + precondition(replacementTimer != completedTimer) + precondition(!lifecycle.isCurrent(completedTimer)) + precondition(lifecycle.isCurrent(replacementTimer)) + precondition(lifecycle.completedSessionID == nil) + + // This is the guard used by delayed Island cleanup. The stale closure + // must leave the replacement timer visible. + if lifecycle.isCurrent(completedTimer) { + islandIsVisible = false + } + precondition(islandIsVisible) + + lifecycle.endSession() + precondition(!lifecycle.isCurrent(replacementTimer)) + } +} diff --git a/tests/test_timer_lifecycle.py b/tests/test_timer_lifecycle.py new file mode 100644 index 00000000..64c8ad1b --- /dev/null +++ b/tests/test_timer_lifecycle.py @@ -0,0 +1,35 @@ +import os +import pathlib +import subprocess +import tempfile +import unittest + + +ROOT = pathlib.Path(__file__).resolve().parents[1] + + +class TimerLifecycleTests(unittest.TestCase): + def test_replacement_session_invalidates_delayed_cleanup_token(self): + with tempfile.TemporaryDirectory() as temporary_directory: + temporary_path = pathlib.Path(temporary_directory) + executable = temporary_path / "timer-lifecycle-regression" + environment = os.environ.copy() + environment["CLANG_MODULE_CACHE_PATH"] = str(temporary_path / "module-cache") + environment["SWIFT_MODULECACHE_PATH"] = str(temporary_path / "module-cache") + subprocess.run( + [ + "swiftc", + str(ROOT / "DynamicIsland/managers/TimerLifecycle.swift"), + str(ROOT / "tests/TimerLifecycleRegression.swift"), + "-o", + str(executable), + ], + check=True, + cwd=ROOT, + env=environment, + ) + subprocess.run([str(executable)], check=True, cwd=ROOT) + + +if __name__ == "__main__": + unittest.main()