Browse Source

Fix memory leak for unused session task

onevcat 7 years ago
parent
commit
41b11c0867

+ 1 - 1
Sources/Cache/ImageCache.swift

@@ -164,7 +164,7 @@ open class ImageCache {
     /// Creates an `ImageCache` from a customized `MemoryStorage` and `DiskStorage`.
     ///
     /// - Parameters:
-    ///   - memoryStorage: The `MemoryStorage` object to use in the image cache.
+    ///   - memoryStorage: The `MemoryStorage.Backend` object to use in the image cache.
     ///   - diskStorage: The `DiskStorage.Backend` object to use in the image cache.
     ///   - name: A name used as a part of the bound IO queue.
     public init(

+ 10 - 4
Sources/Networking/ImageDownloader.swift

@@ -219,10 +219,15 @@ open class ImageDownloader {
             onProgress: onProgress, onCompleted: onCompleted, options: options)
 
         // Ready to start download. Add it to session task manager (`sessionHandler`)
-        let dataTask = session.dataTask(with: request)
-        dataTask.priority = options.downloadPriority
 
-        let downloadTask = sessionDelegate.add(dataTask, url: url, callback: callback)
+        let downloadTask: DownloadTask
+        if let existingTask = sessionDelegate.task(for: url) {
+            downloadTask = sessionDelegate.append(existingTask, url: url, callback: callback)
+        } else {
+            let sessionDataTask = session.dataTask(with: request)
+            sessionDataTask.priority = options.downloadPriority
+            downloadTask = sessionDelegate.add(sessionDataTask, url: url, callback: callback)
+        }
 
         let sessionTask = downloadTask.sessionTask
         sessionTask.onTaskDone.delegate(on: self) { (self, done) in
@@ -240,7 +245,8 @@ open class ImageDownloader {
             switch result {
             // Download finished. Now process the data to an image.
             case .success(let (data, response)):
-                let processor = ImageDataProcessor(data: data, callbacks: callbacks, processingQueue: options.processingQueue)
+                let processor = ImageDataProcessor(
+                    data: data, callbacks: callbacks, processingQueue: options.processingQueue)
                 processor.onImageProcessed.delegate(on: self) { (self, result) in
                     // `onImageProcessed` will be called for `callbacks.count` times, with each
                     // `SessionDataTask.TaskCallback` as the input parameter.

+ 29 - 20
Sources/Networking/SessionDelegate.swift

@@ -60,28 +60,33 @@ class SessionDelegate: NSObject {
         lock.lock()
         defer { lock.unlock() }
 
-        // Try to reuse existing task.
-        if let task = tasks[url] {
-            let token = task.addCallback(callback)
-            return DownloadTask(sessionTask: task, cancelToken: token)
-        } else {
-            // Create a new task if necessary.
-            let task = SessionDataTask(task: dataTask)
-            task.onCallbackCancelled.delegate(on: self) { [unowned task] (self, value) in
-                let (token, callback) = value
-
-                let error = KingfisherError.requestError(reason: .taskCancelled(task: task, token: token))
-                task.onTaskDone.call((.failure(error), [callback]))
-                // No other callbacks waiting, we can clear the task now.
-                if !task.containsCallbacks {
-                    let dataTask = task.task
-                    self.remove(dataTask, acquireLock: true)
-                }
+        // Create a new task if necessary.
+        let task = SessionDataTask(task: dataTask)
+        task.onCallbackCancelled.delegate(on: self) { [unowned task] (self, value) in
+            let (token, callback) = value
+
+            let error = KingfisherError.requestError(reason: .taskCancelled(task: task, token: token))
+            task.onTaskDone.call((.failure(error), [callback]))
+            // No other callbacks waiting, we can clear the task now.
+            if !task.containsCallbacks {
+                let dataTask = task.task
+                self.remove(dataTask, acquireLock: true)
             }
-            let token = task.addCallback(callback)
-            tasks[url] = task
-            return DownloadTask(sessionTask: task, cancelToken: token)
         }
+        let token = task.addCallback(callback)
+        tasks[url] = task
+        return DownloadTask(sessionTask: task, cancelToken: token)
+    }
+
+    func append(
+        _ task: SessionDataTask,
+        url: URL,
+        callback: SessionDataTask.TaskCallback) -> DownloadTask
+    {
+        lock.lock()
+        defer { lock.unlock() }
+        let token = task.addCallback(callback)
+        return DownloadTask(sessionTask: task, cancelToken: token)
     }
 
     func remove(_ task: URLSessionTask, acquireLock: Bool) {
@@ -106,6 +111,10 @@ class SessionDelegate: NSObject {
         return sessionTask
     }
 
+    func task(for url: URL) -> SessionDataTask? {
+        return tasks[url]
+    }
+
     func cancelAll() {
         for task in tasks.values {
             task.forceCancel()

+ 1 - 2
Tests/KingfisherTests/KingfisherManagerTests.swift

@@ -665,8 +665,7 @@ class KingfisherManagerTests: XCTestCase {
         _ = manager.retrieveImage(with: .provider(provider)) { result in
             called = true
             XCTAssertNotNil(result.error)
-            if case .imageSettingError(reason: .dataProviderError(let p, let error)) = result.error! {
-                XCTAssertEqual(provider.identifier, p.identifier)
+            if case .imageSettingError(reason: .dataProviderError(_, let error)) = result.error! {
                 XCTAssertTrue(error is SimpleImageDataProvider.E)
             } else {
                 XCTFail()