How to resolve memory cycle in Swift 4 URLSession.downloadTask?

Question

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.

viewController.swift

@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).

Web.swift

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)
    }
}

Answer 1

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?

Answer 2

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.

Answer 3

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()
}