Memory leak due to failed deinitialization of AVPlayer and observers

Question

EDIT in 10/2020: Previous title was "Deinitializer not called in UIViewController of UIPageViewController"

I would like the following deinitializer in my UIViewController(s) (which is part of a UIPageViewController) to remove my playerLayer and set the player to nil so that the memory is not overloaded (since deinit should always be called when the UIViewController is so to say not needed anymore):

deinit {
        
    self.player = nil
    self.playerLayer.removeFromSuperlayer()
    print("deinit")
    
}

To check if the deinit was ever executed I added the print and found that it is never being called. Can someone explain why it is not called? What would you suggest me to do to achieve what I want to do?

EDIT:

Following the instructions in this question suggested by Rob (in the comments) I found that the following function causes the memory leaks. The function is supposed to setup a player if a file can be found in the documents directory.

setupPlayer() function:

//setup video player
func setupPlayer() {
    
    //get name of file on server //self.video is a String containing the URL for a video on a server
    let fileName = URL(string: self.video!)!.lastPathComponent
    
    let path = NSSearchPathForDirectoriesInDomains(.documentDirectory, .userDomainMask, true)[0] as String
    let url = NSURL(fileURLWithPath: path)
    let filePath = url.appendingPathComponent(fileName)?.path
    let fileManager = FileManager.default
    
    if fileManager.fileExists(atPath: filePath!) {
    
        //create file with name on server if not there already
        let paths = NSSearchPathForDirectoriesInDomains(.documentDirectory, .userDomainMask, true)
        if let docDir = paths.first
        {
            
            let appFile  = docDir.appending("/" + fileName)
            let videoFileUrl = URL(fileURLWithPath: appFile)
            
            //player's video
            if self.player == nil {
                
                let playerItemToBePlayed = AVPlayerItem(url: videoFileUrl) //AVPlayerItem(url: videoFileUrl)
                
                self.player = AVPlayer(playerItem: playerItemToBePlayed)
                
                //add sub-layer
                playerLayer = AVPlayerLayer(player: self.player)
                playerLayer.frame = self.view.frame
                self.controlsContainerView.layer.insertSublayer(playerLayer, at: 0)
                
                //when are frames actually rendered (when is video loaded)
                self.player?.addObserver(self, forKeyPath: "currentItem.loadedTimeRanges", options: .new, context:nil)
                
                //loop through video
                NotificationCenter.default.addObserver(forName: .AVPlayerItemDidPlayToEndTime, object: self.player?.currentItem, queue: nil, using: { (_) in
                    DispatchQueue.main.async {
                        self.player?.seek(to: kCMTimeZero)
                        self.player?.play()
                    }
                })
                
            }
            
        }
        
    }

}

pageViewController function (viewcontrollerAfter)

func pageViewController(_ pageViewController: UIPageViewController, viewControllerAfter viewController: UIViewController) -> UIViewController? 
{
    
    let currentIndexString  = (viewController as! MyViewController).index
    let currentIndex        = indec.index(of: currentIndexString!)
    
    //set if so that next page
    if currentIndex! < indec.count - 1 {
    
        //template
        let myViewController = MyViewController()
    
        //enter data into template
        myViewController.index            = self.indec[currentIndex! + 1]
    
        //return template with data
        return myViewController
        
    }
    
    return nil
    
}

EDIT 2:

As you can see there is no traceback, please pay attention to the size of this malloc (top right) and of similarly big mallocs (bottom left).

Answer 1

If we look at the object graph in "Debug Memory Graph", we can see:

We can see that the view controller is captured by the closure (that middle path). We can also see the observer is keeping a strong reference (that bottom path).

Because I turned on "Malloc stack" feature (shown in https://stackoverflow.com/a/30993476/1271826), I could click on the "Closure Captures" and can see the stack trace in the right panel:

(Forgive me that that memory graph is slightly different than the first screen snapshot because I fixed the other memory issue, the observer, as discussed at the end of this answer.)

Anyway, if I click on the highest entry in the stack trace that's in black (i.e. the last bit of my own code in that stack trace), it takes us directly to the offending code:

That draws our attention to your original code:

NotificationCenter.default.addObserver(forName: .AVPlayerItemDidPlayToEndTime, object: self.player?.currentItem, queue: nil, using: { (_) in
    DispatchQueue.main.async {
        self.player?.seek(to: kCMTimeZero)
        self.player?.play()
    }
})

The closure is keeping strong reference to self. You can correct that with:

NotificationCenter.default.addObserver(forName: .AVPlayerItemDidPlayToEndTime, object: player?.currentItem, queue: .main) { [weak self] _ in
    self?.player?.seek(to: kCMTimeZero)
    self?.player?.play()
}

Note, the [weak self] capture list in the closure.

---

By the way, while you don't need to nil your player in deinit, you do need to remove observers. I'd also set a context for your observer so your observerValue(forKeyPath:of:change:context:) can know whether it's something it needs to handle or not.

So that might result in something like:

private var observerContext = 0
private weak var observer: NSObjectProtocol?

func setupPlayer() {
    let fileName = URL(string: video!)!.lastPathComponent

    let fileManager = FileManager.default
    let videoFileUrl = try! fileManager.url(for: .documentDirectory, in: .userDomainMask, appropriateFor: nil, create: false)
        .appendingPathComponent(fileName)

    if fileManager.fileExists(atPath: videoFileUrl.path), player == nil {
        let playerItemToBePlayed = AVPlayerItem(url: videoFileUrl)

        player = AVPlayer(playerItem: playerItemToBePlayed)

        //add sub-layer
        playerLayer = AVPlayerLayer(player: player)
        playerLayer.frame = view.bounds
        controlsContainerView.layer.insertSublayer(playerLayer, at: 0)

        //when are frames actually rendered (when is video loaded)
        player?.addObserver(self, forKeyPath: "currentItem.loadedTimeRanges", options: .new, context: &observerContext)

        //loop through video
        observer = NotificationCenter.default.addObserver(forName: .AVPlayerItemDidPlayToEndTime, object: player?.currentItem, queue: .main) { [weak self] _ in
            self?.player?.seek(to: kCMTimeZero)
            self?.player?.play()
        }
    }
}

deinit {
    print("deinit")

    // remove loadedTimeRanges observer 

    player?.removeObserver(self, forKeyPath: "currentItem.loadedTimeRanges")

    // remove AVPlayerItemDidPlayToEndTime observer

    if let observer = observer {
        NotificationCenter.default.removeObserver(observer)
    }
}

// note, `observeValue` should check to see if this is something 
// this registered for or whether it should pass it along to `super`

override func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey : Any]?, context: UnsafeMutableRawPointer?) {
    guard context == &observerContext else {
        super.observeValue(forKeyPath: keyPath, of: object, change: change, context: context)
        return
    }

    // do something
}