Fix race condition when flagging site as unsupported for application passwords (#16237)

Author: itsmeichigo

Modules/Sources/NetworkingCore/Network/AlamofireNetworkErrorHandler.swift

@@ -85,15 +85,23 @@ final class AlamofireNetworkErrorHandler {
         originalRequest: URLRequestConvertible,
         failure: Error?
     ) {
-        let retriedRequestIndex = retriedJetpackRequests.firstIndex { retriedRequest in
-            let urlRequest = try? originalRequest.asURLRequest()
-            let retriedRequest = try? retriedRequest.request.asURLRequest()
-            return urlRequest == retriedRequest
-        }
+        let retriedRequest: RetriedJetpackRequest? = queue.sync(flags: .barrier) { [weak self] in
+            guard let self else { return nil }
+            guard let urlRequest = try? originalRequest.asURLRequest() else { return nil }
+            let retriedRequestIndex = _retriedJetpackRequests.firstIndex { retriedRequest in
+                guard let retriedURLRequest = try? retriedRequest.request.asURLRequest() else {
+                    return false
+                }
+                return urlRequest.url == retriedURLRequest.url &&
+                       urlRequest.httpMethod == retriedURLRequest.httpMethod
+            }
 
-        guard let index = retriedRequestIndex else { return }
+            guard let index = retriedRequestIndex else { return nil }
+
+            return _retriedJetpackRequests.remove(at: index)
+        }
 
-        let retriedRequest = retriedJetpackRequests.remove(at: index)
+        guard let retriedRequest else { return }
 
         if failure == nil {
             let siteID = retriedRequest.request.siteID
@@ -141,10 +149,15 @@ final class AlamofireNetworkErrorHandler {
     }
 
     func isRequestRetried(_ request: URLRequestConvertible) -> Bool {
-        retriedJetpackRequests.contains { retriedRequest in
-            let urlRequest = try? request.asURLRequest()
-            let currentItem = try? retriedRequest.request.asURLRequest()
-            return currentItem == urlRequest
+        guard let urlRequest = try? request.asURLRequest() else {
+            return false
+        }
+        return retriedJetpackRequests.contains { retriedRequest in
+            guard let currentItem = try? retriedRequest.request.asURLRequest() else {
+                return false
+            }
+            return currentItem.url == urlRequest.url &&
+                   currentItem.httpMethod == urlRequest.httpMethod
         }
     }
 

Modules/Tests/NetworkingTests/Network/AlamofireNetworkErrorHandlerTests.swift

@@ -560,6 +560,52 @@ final class AlamofireNetworkErrorHandlerTests: XCTestCase {
         // Then
         XCTAssertTrue(userDefaults.applicationPasswordUnsupportedList.keys.contains(String(siteID)))
     }
+
+    func test_concurrent_flagSiteAsUnsupportedForAppPasswordIfNeeded_no_race_condition() {
+        // Given - test for the race condition fix where multiple threads
+        // try to remove the same item simultaneously
+        let expectation = XCTestExpectation(description: "All concurrent flag operations complete without crash")
+        let threadCount = 50
+        let siteID: Int64 = 999
+        let jetpackRequest = createJetpackRequest(siteID: siteID)
+        let restRequest = createRESTRequest()
+        let error = createNetworkError()
+
+        expectation.expectedFulfillmentCount = threadCount
+
+        // Add a single request to the retry list
+        _ = errorHandler.shouldRetryJetpackRequest(
+            originalRequest: jetpackRequest,
+            convertedRequest: restRequest,
+            failure: error
+        )
+
+        // When - multiple threads concurrently try to flag and remove the SAME item
+        // This would cause a race condition in the old code where:
+        // 1. Thread A reads the array, finds index 0
+        // 2. Thread B reads the array, finds index 0
+        // 3. Thread A removes at index 0 (succeeds)
+        // 4. Thread B tries to remove at index 0 (crashes - array is now empty)
+        let group = DispatchGroup()
+        for _ in 0..<threadCount {
+            group.enter()
+            DispatchQueue.global().async {
+                // All threads try to remove the same request concurrently
+                self.errorHandler.flagSiteAsUnsupportedForAppPasswordIfNeeded(
+                    originalRequest: jetpackRequest,
+                    failure: nil
+                )
+                group.leave()
+                expectation.fulfill()
+            }
+        }
+
+        // Wait for all threads to complete
+        group.wait()
+
+        // Then - no crashes should occur (especially no EXC_BREAKPOINT from array index out of bounds)
+        wait(for: [expectation], timeout: 5.0)
+    }
 }
 
 // MARK: - Helper Methods