Fix libdispatch over-resume crash in DispatchTimerSource.deinit

AudioStreaming/Core/Helpers/DispatchTimerSource.swift

@@ -11,14 +11,19 @@ import Foundation
 final class DispatchTimerSource {
     private var handler: (() -> Void)?
     private let timer: DispatchSourceTimer
-    var state: SourceState = .suspended
+    private let lock = UnfairLock()
+    private var _state: SourceState = .suspended
 
     /// The state of the timer
     enum SourceState {
         case activated
         case suspended
     }
 
+    /// Read-only: the suspend/resume balance is only correct when the state check and the state
+    /// change happen together under the lock, which is what `activate()`/`suspend()` do.
+    var state: SourceState { lock.withLock { _state } }
+
     var isRunning: Bool {
         state == .activated
     }
@@ -39,8 +44,19 @@ final class DispatchTimerSource {
     deinit {
         timer.setEventHandler(handler: nil)
         timer.cancel()
-        // balance called of cancel/resume to avoid crashes
-        timer.resume()
+        // A suspended dispatch source traps if it is released without being resumed first — but
+        // resuming one that is already running is an over-resume, which traps just the same. The
+        // balancing resume therefore has to be conditional: this object is routinely deallocated
+        // while its timer is still activated, e.g. a pending retry whose owner goes away.
+        let needsResume = lock.withLock { () -> Bool in
+            let wasSuspended = _state == .suspended
+            _state = .activated
+            return wasSuspended
+        }
+
+        if needsResume {
+            timer.resume()
+        }
     }
 
     /// Adds an event handler to the timer.
@@ -58,16 +74,20 @@ final class DispatchTimerSource {
 
     /// Activates the timer, if needed
     func activate() {
-        if state == .activated { return }
-        state = .activated
-        timer.resume()
+        lock.withLock {
+            guard _state == .suspended else { return }
+            _state = .activated
+            timer.resume()
+        }
     }
 
     /// Suspends the timer, if needed.
     func suspend() {
-        if state == .suspended { return }
-        state = .suspended
-        timer.suspend()
+        lock.withLock {
+            guard _state == .activated else { return }
+            _state = .suspended
+            timer.suspend()
+        }
     }
 
     func schedule(interval: DispatchTimeInterval, repeats: Bool) {

AudioStreamingTests/Core/DispatchTimerSourceTests.swift

@@ -66,6 +66,34 @@ class DispatchTimerSourceTests: XCTestCase {
         timerSource?.removeHandler()
     }
 
+    func test_DispatchTimerSource_Can_Be_Deallocated_While_Activated() {
+        // `deinit` used to resume unconditionally to balance a suspended source. When the source
+        // was still activated — a pending retry whose owner is torn down — that extra resume is an
+        // over-resume and libdispatch traps on it. Reaching the end of this test is the assertion.
+        var source: DispatchTimerSource? = DispatchTimerSource(interval: .milliseconds(100), queue: dispatchQueue)
+        source?.activate()
+        XCTAssertTrue(source!.isRunning)
+
+        source = nil
+        XCTAssertNil(source)
+    }
+
+    func test_DispatchTimerSource_Repeated_Activate_And_Suspend_Stay_Balanced() {
+        // Each activate/suspend must move the suspend count exactly once, no matter how often the
+        // no-op path is taken.
+        timerSource?.activate()
+        timerSource?.activate()
+        XCTAssertTrue(timerSource!.isRunning)
+
+        timerSource?.suspend()
+        timerSource?.suspend()
+        XCTAssertFalse(timerSource!.isRunning)
+
+        timerSource?.activate()
+        XCTAssertTrue(timerSource!.isRunning)
+        timerSource?.suspend()
+    }
+
     func test_HandlerIsExecuted_On_The_Specified_Queue() {
         let expectaction = expectation(description: "fired")