Răsfoiți Sursa

Remove ActorBox and harden background task cleanup

onevcat 1 lună în urmă
părinte
comite
17cbf2dda9

+ 28 - 16
Sources/Cache/ImageCache.swift

@@ -868,31 +868,43 @@ open class ImageCache: @unchecked Sendable {
     @objc public func backgroundCleanExpiredDiskCache() {
         // if 'sharedApplication()' is unavailable, then return
         guard let sharedApplication = KingfisherWrapper<UIApplication>.shared else { return }
-        
-        let taskActor = ActorBox<UIBackgroundTaskIdentifier?>(nil)
-        
-        let createdTask = sharedApplication.beginBackgroundTask(withName: "Kingfisher:backgroundCleanExpiredDiskCache") {
-            Task {
-                guard let bgTask = await taskActor.value, bgTask != .invalid else { return }
-                sharedApplication.endBackgroundTask(bgTask)
-                await taskActor.setValue(.invalid)
+
+        actor BackgroundTaskState {
+            private var value: UIBackgroundTaskIdentifier? = nil
+
+            func setValue(_ newValue: UIBackgroundTaskIdentifier) {
+                value = newValue
+            }
+
+            func takeValidValueAndInvalidate() -> UIBackgroundTaskIdentifier? {
+                guard let task = value, task != .invalid else { return nil }
+                value = .invalid
+                return task
             }
         }
-        
-        cleanExpiredDiskCache {
-            Task {
-                guard let bgTask = await taskActor.value, bgTask != .invalid else { return }
+
+        let taskState = BackgroundTaskState()
+
+        func endBackgroundTaskIfNeeded() {
+            Task { @MainActor in
+                guard let bgTask = await taskState.takeValidValueAndInvalidate() else { return }
                 #if compiler(>=6)
                 sharedApplication.endBackgroundTask(bgTask)
                 #else
                 await sharedApplication.endBackgroundTask(bgTask)
                 #endif
-                await taskActor.setValue(.invalid)
             }
         }
-        
-        Task {
-            await taskActor.setValue(createdTask)
+
+        let createdTask = sharedApplication.beginBackgroundTask(
+            withName: "Kingfisher:backgroundCleanExpiredDiskCache",
+            expirationHandler: endBackgroundTaskIfNeeded
+        )
+
+        Task { await taskState.setValue(createdTask) }
+
+        cleanExpiredDiskCache {
+            endBackgroundTaskIfNeeded()
         }
     }
 #endif

+ 0 - 11
Sources/Utility/Box.swift

@@ -32,14 +32,3 @@ class Box<T> {
         self.value = value
     }
 }
-
-actor ActorBox<T> {
-    var value: T
-    init(_ value: T) {
-        self.value = value
-    }
-    
-    func setValue(_ value: T) {
-        self.value = value
-    }
-}

+ 15 - 32
Tests/KingfisherTests/KingfisherManagerTests.swift

@@ -1243,31 +1243,25 @@ class KingfisherManagerTests: XCTestCase {
         
         stub(url, data: testImageData)
         
-        let task = ActorBox<DownloadTask?>(nil)
-        
-        let called = ActorBox(false)
+        let task = LockIsolated<DownloadTask?>(nil)
+        let callbackCount = LockIsolated(0)
         
         let t: DownloadTask? = manager.retrieveImage(with: url) { result in
-            Task {
-                let calledResult = await called.value
-                XCTAssertFalse(calledResult)
-                XCTAssertNotNil(result.value?.image)
-                
-                if !calledResult {
-                    Task {
-                        await task.value?.cancel()
-                    }
-                    DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) {
-                        exp.fulfill()
-                    }
-                } else {
-                    XCTFail("Callback should not be invoked again.")
-                }
+            let count = callbackCount.withValue { value in
+                value += 1
+                return value
+            }
+
+            XCTAssertEqual(count, 1, "Callback should not be invoked again.")
+            XCTAssertNotNil(result.value?.image)
+
+            task.value?.cancel()
+            DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) {
+                exp.fulfill()
             }
         }
-        Task {
-            await task.setValue(t)
-        }
+
+        task.setValue(t)
         waitForExpectations(timeout: 3, handler: nil)
     }
 
@@ -1789,17 +1783,6 @@ struct SimpleImageDataProvider: ImageDataProvider, @unchecked Sendable {
     struct E: Error {}
 }
 
-actor ActorBox<T> {
-    var value: T
-    init(_ value: T) {
-        self.value = value
-    }
-    
-    func setValue(_ value: T) {
-        self.value = value
-    }
-}
-
 actor ActorArray<Element> {
     var value: [Element]
     init(_ value: [Element]) {

+ 1 - 7
Tests/KingfisherTests/RetryStrategyTests.swift

@@ -215,7 +215,6 @@ class RetryStrategyTests: XCTestCase {
 
     func testDelayRetryStrategyDidRetried() {
         let exp = expectation(description: #function)
-        let called = ActorBox(false)
         let source = Source.network(URL(string: "url")!)
         let retry = DelayRetryStrategy(maxRetryCount: 3, retryInterval: .seconds(0))
         let context = RetryContext(
@@ -227,12 +226,7 @@ class RetryStrategyTests: XCTestCase {
                 XCTFail("The decision should be `retry`.")
                 return
             }
-            Task {
-                await called.setValue(true)
-                let result = await called.value
-                XCTAssertTrue(result)
-                exp.fulfill()
-            }
+            exp.fulfill()
         }
 
         waitForExpectations(timeout: 3, handler: nil)