Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading