Fix retryContext and its tests for concurrency

Sources/General/KingfisherManager.swift

@@ -356,7 +356,7 @@ public class KingfisherManager: @unchecked Sendable {
                     retryStrategy.retry(context: context) { decision in
                         switch decision {
                         case .retry(let userInfo):
-                            retryContext?.userInfo = userInfo
+                            context.userInfo = userInfo
                             startNewRetrieveTask(with: source, downloadTaskUpdated: downloadTaskUpdated)
                         case .stop:
                             failCurrentSource(currentSource, with: error)

Sources/Networking/ImageDownloader.swift

@@ -522,7 +522,7 @@ open class ImageDownloader: @unchecked Sendable {
         with url: URL,
         options: KingfisherOptionsInfo? = nil,
         progressBlock: DownloadProgressBlock? = nil,
-        completionHandler: ((Result<ImageLoadingResult, KingfisherError>) -> Void)? = nil) -> DownloadTask?
+        completionHandler: (@Sendable (Result<ImageLoadingResult, KingfisherError>) -> Void)? = nil) -> DownloadTask?
     {
         var info = KingfisherParsedOptionsInfo(options)
         if let block = progressBlock {
@@ -547,7 +547,7 @@ open class ImageDownloader: @unchecked Sendable {
     open func downloadImage(
         with url: URL,
         options: KingfisherOptionsInfo? = nil,
-        completionHandler: ((Result<ImageLoadingResult, KingfisherError>) -> Void)? = nil) -> DownloadTask
+        completionHandler: (@Sendable (Result<ImageLoadingResult, KingfisherError>) -> Void)? = nil) -> DownloadTask
     {
         downloadImage(
             with: url,

Sources/Networking/RetryStrategy.swift

@@ -82,7 +82,7 @@ public protocol RetryStrategy: Sendable {
     /// - Parameters:
     ///   - context: The retry context containing information of the current retry attempt.
     ///   - retryHandler: A block you need to call with a decision on whether the retry should happen or not.
-    func retry(context: RetryContext, retryHandler: @escaping (RetryDecision) -> Void)
+    func retry(context: RetryContext, retryHandler: @escaping @Sendable (RetryDecision) -> Void)
 }
 
 /// A retry strategy that guides Kingfisher to perform retry operation with some delay.
@@ -143,7 +143,7 @@ public struct DelayRetryStrategy: RetryStrategy {
         self.retryInterval = retryInterval
     }
 
-    public func retry(context: RetryContext, retryHandler: @escaping (RetryDecision) -> Void) {
+    public func retry(context: RetryContext, retryHandler: @escaping @Sendable (RetryDecision) -> Void) {
         // Retry count exceeded.
         guard context.retriedCount < maxRetryCount else {
             retryHandler(.stop)

Tests/KingfisherTests/KingfisherManagerTests.swift

@@ -1367,3 +1367,14 @@ actor ActorBox<T> {
         self.value = value
     }
 }
+
+actor ActorArray<Element> {
+    var value: [Element]
+    init(_ value: [Element]) {
+        self.value = value
+    }
+    
+    func append(_ newElement: Element) {
+        value.append(newElement)
+    }
+}

Tests/KingfisherTests/RetryStrategyTests.swift

@@ -103,12 +103,15 @@ class RetryStrategyTests: XCTestCase {
     }
 
     func testDelayRetryStrategyExceededCount() {
-
-        var blockCalled: [Bool] = []
+        let exp = expectation(description: #function)
+        let blockCalled: ActorArray<Bool> = ActorArray([])
 
         let source = Source.network(URL(string: "url")!)
         let retry = DelayRetryStrategy(maxRetryCount: 3, retryInterval: .seconds(0))
 
+        let group = DispatchGroup()
+        
+        group.enter()
         let context1 = RetryContext(
             source: source,
             error: .responseError(reason: .URLSessionError(error: E()))
@@ -119,9 +122,13 @@ class RetryStrategyTests: XCTestCase {
                 return
             }
             XCTAssertNil(userInfo)
-            blockCalled.append(true)
+            Task {
+                await blockCalled.append(true)
+                group.leave()
+            }
         }
 
+        group.enter()
         let context2 = RetryContext(
             source: source,
             error: .responseError(reason: .URLSessionError(error: E()))
@@ -134,21 +141,34 @@ class RetryStrategyTests: XCTestCase {
                 XCTFail("The decision should be `stop`")
                 return
             }
-            blockCalled.append(true)
+            Task {
+                await blockCalled.append(true)
+                group.leave()
+            }
         }
-
-        XCTAssertEqual(blockCalled.count, 2)
-        XCTAssertTrue(blockCalled.allSatisfy { $0 })
+        
+        group.notify(queue: .main) {
+            Task {
+                let result = await blockCalled.value
+                XCTAssertEqual(result.count, 2)
+                XCTAssertTrue(result.allSatisfy { $0 })
+                exp.fulfill()
+            }
+        }
+        waitForExpectations(timeout: 1, handler: nil)
     }
 
     func testDelayRetryStrategyNotRetryForErrorReason() {
+        let exp = expectation(description: #function)
         // Only non-user cancel error && response error should be retied.
-        var blockCalled: [Bool] = []
+        let blockCalled: ActorArray<Bool> = ActorArray([])
         let source = Source.network(URL(string: "url")!)
         let retry = DelayRetryStrategy(maxRetryCount: 3, retryInterval: .seconds(0))
 
         let task = URLSession.shared.dataTask(with: URL(string: "url")!)
 
+        let group = DispatchGroup()
+        group.enter()
         let context1 = RetryContext(
             source: source,
             error: .requestError(reason: .taskCancelled(task: .init(task: task), token: .init()))
@@ -158,9 +178,13 @@ class RetryStrategyTests: XCTestCase {
                 XCTFail("The decision should be `stop` if user cancelled the task.")
                 return
             }
-            blockCalled.append(true)
+            Task {
+                await blockCalled.append(true)
+                group.leave()
+            }
         }
 
+        group.enter()
         let context2 = RetryContext(
             source: source,
             error: .cacheError(reason: .imageNotExisting(key: "any_key"))
@@ -170,15 +194,25 @@ class RetryStrategyTests: XCTestCase {
                 XCTFail("The decision should be `stop` if the error type is not response error.")
                 return
             }
-            blockCalled.append(true)
+            Task {
+                await blockCalled.append(true)
+                group.leave()
+            }
         }
-
-        XCTAssertEqual(blockCalled.count, 2)
-        XCTAssertTrue(blockCalled.allSatisfy { $0 })
+        
+        group.notify(queue: .main) {
+            Task {
+                let result = await blockCalled.value
+                XCTAssertEqual(result.count, 2)
+                XCTAssertTrue(result.allSatisfy { $0 })
+                exp.fulfill()
+            }
+        }
+        waitForExpectations(timeout: 1, handler: nil)
     }
 
     func testDelayRetryStrategyDidRetried() {
-        var called = false
+        let called = ActorBox(false)
         let source = Source.network(URL(string: "url")!)
         let retry = DelayRetryStrategy(maxRetryCount: 3, retryInterval: .seconds(0))
         let context = RetryContext(
@@ -190,10 +224,16 @@ class RetryStrategyTests: XCTestCase {
                 XCTFail("The decision should be `retry`.")
                 return
             }
-            called = true
+            Task {
+                await called.setValue(true)
+            }
         }
 
-        XCTAssertTrue(called)
+        Task {
+            let result = await called.value
+            XCTAssertTrue(result)
+        }
+        
     }
 }