Fix Auth APNs token timeout for concurrent waiters

FirebaseAuth/CHANGELOG.md

@@ -1,4 +1,6 @@
 # 13.0.0
+- [fixed] Fixed a timeout race in the APNs token manager that could leave concurrent phone
+  authentication requests waiting indefinitely.
 - [changed] Replaced a force-unwrapped error with a safe fallback during Game
   Center authentication. (#16136)
 

FirebaseAuth/Sources/Swift/SystemService/AuthAPNSTokenManager.swift

@@ -56,22 +56,21 @@
       }
       if pendingCallbacks.count > 0 {
         pendingCallbacks.append(callback)
-        // TODO(ncooke3): This is likely a bug in that the async wrapper method
-        // cannot make forward progress.
         return
       }
+      pendingRequestGeneration += 1
+      let requestGeneration = pendingRequestGeneration
       pendingCallbacks = [callback]
 
       DispatchQueue.main.async {
         self.application.registerForRemoteNotifications()
       }
-      let applicableCallbacks = pendingCallbacks
       let deadline = DispatchTime.now() + timeout
       kAuthGlobalWorkQueue.asyncAfter(deadline: deadline) {
-        // Only cancel if the pending callbacks remain the same, i.e., not triggered yet.
-        if applicableCallbacks.count == self.pendingCallbacks.count {
-          self.callback(.failure(AuthErrorUtils.missingAppTokenError(underlyingError: nil)))
-        }
+        // Only cancel if this is still the active request and it has not completed.
+        guard requestGeneration == self.pendingRequestGeneration,
+              !self.pendingCallbacks.isEmpty else { return }
+        self.callback(.failure(AuthErrorUtils.missingAppTokenError(underlyingError: nil)))
       }
     }
 
@@ -124,6 +123,7 @@
     /// Enable unit test faking.
     var application: AuthAPNSTokenApplication
     private var pendingCallbacks: [(Result<AuthAPNSToken, Error>) -> Void] = []
+    private var pendingRequestGeneration = 0
 
     private func callback(_ result: Result<AuthAPNSToken, Error>) {
       let pendingCallbacks = self.pendingCallbacks

FirebaseAuth/Tests/Unit/AuthAPNSTokenManagerTests.swift

@@ -164,6 +164,119 @@
       waitForExpectations(timeout: 5)
     }
 
+    /** @fn testMultipleCallbacksTimeout
+        @brief Tests all pending callbacks are called when registration times out.
+     */
+    func testMultipleCallbacksTimeout() throws {
+      manager = AuthAPNSTokenManager(
+        withApplication: fakeApplication!,
+        timeout: kRegistrationTimeout
+      )
+      let manager = try XCTUnwrap(manager)
+      let expectation = self.expectation(description: #function)
+      expectation.expectedFulfillmentCount = 2
+      let callbackCount = UnfairLock(0)
+
+      for _ in 0 ..< 2 {
+        manager.getTokenInternal { result in
+          switch result {
+          case let .success(token):
+            XCTFail("Unexpected success: \(token)")
+          case let .failure(error):
+            XCTAssertEqual(
+              error as NSError,
+              AuthErrorUtils.missingAppTokenError(underlyingError: nil) as NSError
+            )
+          }
+          callbackCount.withLock { $0 += 1 }
+          expectation.fulfill()
+        }
+      }
+
+      waitForExpectations(timeout: 2)
+      XCTAssertEqual(callbackCount.value(), 2)
+    }
+
+    /** @fn testOldTimeoutDoesNotCancelNewRequest
+        @brief Tests a timeout from a cancelled request cannot complete a later request.
+     */
+    func testOldTimeoutDoesNotCancelNewRequest() throws {
+      let timeout: TimeInterval = 1
+      manager = AuthAPNSTokenManager(
+        withApplication: fakeApplication!,
+        timeout: timeout
+      )
+      let manager = try XCTUnwrap(manager)
+      let expectedError = error
+
+      let firstRequestCanceled = expectation(description: "firstRequestCanceled")
+      let secondCallbackCalled = UnfairLock(false)
+
+      manager.getTokenInternal { result in
+        switch result {
+        case let .success(token):
+          XCTFail("Unexpected success: \(token)")
+        case let .failure(error):
+          XCTAssertEqual(error as NSError, expectedError)
+        }
+        firstRequestCanceled.fulfill()
+      }
+
+      // Start the second request before the first request's timeout fires, so the old
+      // timeout would incorrectly match it if requests were compared by callback count.
+      let noEarlyCallback = expectation(description: "noEarlyCallback")
+
+      DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) {
+        manager.cancel(withError: expectedError)
+
+        manager.getTokenInternal { result in
+          secondCallbackCalled.withLock { $0 = true }
+
+          switch result {
+          case let .success(token):
+            XCTFail("Unexpected success: \(token)")
+          case let .failure(error):
+            XCTAssertEqual(error as NSError, expectedError)
+          }
+        }
+      }
+
+      DispatchQueue.main.asyncAfter(deadline: .now() + 1.1) {
+        XCTAssertFalse(secondCallbackCalled.value())
+        manager.cancel(withError: expectedError)
+        noEarlyCallback.fulfill()
+      }
+
+      waitForExpectations(timeout: 2)
+    }
+
+    /** @fn testGetTokenTimeoutWithMultipleWaiters
+        @brief Tests concurrent async callers complete when registration times out.
+     */
+    func testGetTokenTimeoutWithMultipleWaiters() async throws {
+      manager = AuthAPNSTokenManager(
+        withApplication: fakeApplication!,
+        timeout: kRegistrationTimeout
+      )
+      let manager = try XCTUnwrap(manager)
+
+      async let firstResult = getTokenResult(from: manager)
+      async let secondResult = getTokenResult(from: manager)
+
+      let results = await [firstResult, secondResult]
+      for result in results {
+        switch result {
+        case let .success(token):
+          XCTFail("Unexpected success: \(token)")
+        case let .failure(error):
+          XCTAssertEqual(
+            error as NSError,
+            AuthErrorUtils.missingAppTokenError(underlyingError: nil) as NSError
+          )
+        }
+      }
+    }
+
     /** @fn testCancel
         @brief Tests cancelling the pending callbacks.
      */
@@ -211,5 +324,14 @@
         registerCalled = true
       }
     }
+
+    private func getTokenResult(from manager: AuthAPNSTokenManager) async ->
+      Result<AuthAPNSToken, Error> {
+      do {
+        return try .success(await manager.getToken())
+      } catch {
+        return .failure(error)
+      }
+    }
   }
 #endif