Reputation: 12737
I have a basic viewController
with a single button that when tapped, calls a method to start downloading an image from a given valid url.
I was very careful of passing any strong pointers to the viewcontroller. And I am able to dismiss the viewcontroller away and go back to the presenting controller without any issues. However, objects instances of Web()
that are created by the viewcontroller, are not deallocated, even if the viewcontroller's deinit
is called.
What am I doing wrong?
BTW: the file download is started, progress is reported and file location is reported as well. However, the object is never deallocated once the file is finished downloading.
@IBAction func buttonTapped(_ sender: UIButton) {
//first create an instance of Web class (download helper)
let web = Web(url: "https://www.google.com.sa/images/branding/googlelogo/2x/googlelogo_color_120x44dp.png")
// all call back closures have no reference to self
web.downloadFile(
finishedHandler: {fileLocation in print("file stored to \(fileLocation)")},
progressHandler: {bytes,total in print("written \(bytes) out of \(total)")},
errorHandler: {msg in print("error: \(msg)")}
)
}
And I have defined Web()
as NSObject
which conforms to URLSessionDownloadDelegate
to take advantage of delegate event methods (didFinishDownload
/ didWriteBytes
, etc).
import UIKit
class Web : NSObject {
var urlToDownload : String?
// the following variables are references to closures passed to the object.
var progressCallback : ((_ bytesWritten:Int64, _ totalExpectedBytes: Int64)->())?
var finishedCallback : ((_ fileLocation: String)->())?
static var instanceCount = 0 // keep track of number of instances created
init(url: String) {
Web.instanceCount += 1
urlToDownload = url
print(" new instance of Web created. Total : \(Web.instanceCount)")
}
deinit {
Web.instanceCount -= 1
print("Web instance deallocated. Remaining: \(Web.instanceCount)")
}
}
and I added the file download logic as extension to the same file:
extension Web : URLSessionDownloadDelegate {
func downloadFile(
finishedHandler: @escaping (_ fileLocation:String)->(),
progressHandler: @escaping (_ bytesWritten:Int64, _ totalBytes: Int64)->(),
errorHandler: @escaping (_ errorMsg:String)->()) {
// we need to capture the closure because, these will
// be called once the delegate methods are triggered
self.progressCallback = progressHandler
self.finishedCallback = finishedHandler
if let url = URL(string: self.urlToDownload!) {
let session = URLSession(
configuration: .default,
delegate: self,
delegateQueue: nil)
let task = session.downloadTask(with: url)
task.resume()
}
}
// MARK :- Delegate methods
func urlSession(_ session: URLSession, downloadTask: URLSessionDownloadTask, didFinishDownloadingTo location: URL) {
// call the closure if it still exists
self.finishedCallback?(location.absoluteString)
}
func urlSession(_ session: URLSession, downloadTask: URLSessionDownloadTask, didWriteData bytesWritten: Int64, totalBytesWritten: Int64, totalBytesExpectedToWrite: Int64) {
// report progress by calling captured closure, if exists
self.progressCallback?(totalBytesWritten,totalBytesExpectedToWrite)
}
}
Upvotes: 9
Views: 1948
Reputation: 535249
The problem is that a URLSession retains its delegate, so storing a strong reference to the URLSession in the same object that acts as its delegate
is circular.
As you've been told, you can break the strong reference by invalidating the URLSession. But in general the simple solution is: don't create the circularity in the first place.
Upvotes: 4
Reputation: 3465
The problem you can clearly see when debugging using the Memory Graph Debugger is that the object Web
is being held by the URLSession
object which holds the CFXURLCache
which in turn holds the URLSession
back. They have strong references to each other. So only when the session or cache gets freed from memory, the Web
object will be freed up from memory.
The SOLUTION is simple, you need to invalidate the session to break the cyclic reference between NSURLSession
and CFXURLCache
when your download finishes:
func urlSession(_ session: URLSession, downloadTask: URLSessionDownloadTask, didFinishDownloadingTo location: URL) {
// call the closure if it still exists
self.finishedCallback?(location.absoluteString)
session.finishTasksAndInvalidate()
}
Upvotes: 2
Reputation: 31645
After I traced the issue (using the Memory Graph Debugger), I figured it out that the reason of the issue in this chunk of code (Web class - download file method):
if let url = URL(string: self.urlToDownload!) {
let session = URLSession(
configuration: .default,
delegate: self,
delegateQueue: nil)
let task = session.downloadTask(with: url)
task.resume()
}
Actually there is an issue related to instantiation URLSession
without getting rid of it; Therefore, you have to do a -kind of- manual handling to solve it, calling finishTasksAndInvalidate()
would be appropriate for such an issue:
if let url = URL(string: self.urlToDownload!) {
let session = URLSession(
configuration: .default,
delegate: self,
delegateQueue: nil)
let task = session.downloadTask(with: url)
task.resume()
// here we go:
session.finishTasksAndInvalidate()
}
To make sure it works as expected, I would suggest to add breakpoints to the delegate methods and the deinit
in the Web class, you should see it works as expected (the delegate method would work and then the deinit
would be called).
Furthermore:
If you would like to know more about how to work with the Memory Graph Debugger, you could check: How to debug memory leaks when Leaks instrument does not show them?
Upvotes: 12