4

I have a childVC(vc3) inside a parentVC(vc2) inside another parentVC(vc1). I'm doing it this way for animation purposes.

What happens is I add vc3 as a child to vc2. I have a collectionView that pushes on vc1. Once vc1 is on scene vc2 is added to it. When I pop vc1 off the stack and go back to the collectionView the deinit in vc1 gets called however the deinit in vc2 never gets called.

Is the deinit in vc2 supposed to get a called even though it's a child of vc1? Or is it possibly because the thirdVC is creating a strong reference to itself inside of the secondVC?

SecondVC with the ThirdVC added inside of it:

class SecondController: UIViewController {

override func viewDidLoad() {
        super.viewDidLoad()

        let thirdVC = ThirdController()
        addChildViewController(thirdVC)
        view.addSubview(thirdVC.view)
        thirdVC.didMove(toParentViewController: self)
}

 // this never runs when the firstVC is popped off the stack
deinit{
     print("the secondVC has be deallocated")
}
}

CollectionView:

func collectionView(_ collectionView: UICollectionView, didSelectItemAt indexPath: IndexPath) {

        let firstVC = FirstController()
        navigationController?.pushViewController(firstVC, animated: true)
    }

FirstVC:

class FirstController: UIViewController {

override func viewDidLoad() {
        super.viewDidLoad()

        let secondVC = SecondController()
        addChildViewController(secondVC)
        view.addSubview(secondVC.view)
        secondVC.didMove(toParentViewController: self)
}

// this runs when popped off the stack
deinit{
     print("the firstVC has be deallocated")
}
}
Lance Samaria
  • 17,576
  • 18
  • 108
  • 256
  • 2
    It seems to be a strong reference issue, But, the `UIViewController.parent` reference is weak. So, I think it's not the case that vc3 is keeping a strong reference to vc2. Delegates are weak, so it shouldn't be the case as well. Are you retrieving those view controllers in any other place of your code? – Bruno Pinheiro Mar 14 '18 at 13:55
  • @BrunoPinheiro hey thanks for the help. The secondVC will be accessed in other places because it has the animation capabilities inside of it. For eg I have a tabBarController, on the first tab there is a collectionView that pushes on vc1 which means vc2 will be on scene also (because it’s a child of vc1). If I press another tab on the tabBar, that will also have a collectionView that can push on another vc (like zeroVC) which also will have the secondVC as a child. Basically both tabs will now show the secondVC as children of their parents -firstVC and zeroVC. – Lance Samaria Mar 14 '18 at 14:01
  • @LanceSamaria You're using same instance of `secondVC` in more than one place? Are you sure that there are no other strong references to the `secondVC` ? – badhanganesh Mar 14 '18 at 14:04
  • @BadhanGanesh yep, no where, it's a small project at this point so the secondVC is only instantiated in those 2 places -firstVc and zeroVC. The thing is even if zeroVC hasn't been pushed on and the only thing that has a copy of the secondVC is the firstVC, the secondVC deinit never gets called when the firstVC is popped of. I even ran a global search using Find in the navigator and the only place that instantiates it is the firstVC (I removed it from the zeroVC). – Lance Samaria Mar 14 '18 at 14:10
  • Use Instruments to see what is keeping a reference to the view controller. – rmaddy Mar 14 '18 at 14:19
  • @rmaddy from what you suggested and what BadhanGanesh asked it sounds like there must be a problem somewhere else. I'll check but I just used cmmd +4 and typed SecondContoller() into the Find field and the only thing that popped up was where I instantiated it inside the firstVC's viewDidLoad. I'll have to use instruments thanks – Lance Samaria Mar 14 '18 at 14:23
  • @LanceSamaria Can you show what is inside your `ThirdController `? – badhanganesh Mar 14 '18 at 14:31
  • @Badhan Ganesh I would but there are a bunch of methods and other things inside of it, the third vc is actually a AVPlayerController: playerItem = AVPlayerItem(asset: asset, automaticallyLoadedAssetKeys: assetKeys) player = AVPlayer(playerItem: playerItem!) let thirdVC = AVPlayerViewController() thirdVC.player = player addChildViewController(thirdVC) view.addSubview(thirdVC.view) thirdVC.didMove(toParentViewController: self) player?.play() etc... Too much code to add – Lance Samaria Mar 14 '18 at 14:39
  • 1
    @LanceSamaria Okay. Are you using closures that reference your `secondVC` instance strongly? Or are you using any notification observers that you fail to remove them when not needed? – badhanganesh Mar 14 '18 at 14:48
  • @LanceSamaria - Good point from Badhan. I think we need to have more details from your code. I would suggest you to start a new empty project, and add the view controllers step by step. For example: Start by showing only FirstController, then, add SecondController. Test the deinit functions. Add the ThirdController. Test deinit again. Maybe it can help you get closer to the problem. – Bruno Pinheiro Mar 14 '18 at 14:58
  • @Badhan Ganesh I the few closures that I use I use [weak self] inside of them. I use KVOand remove the observers in viewDidDisappear. I have a few Notifications but I don't remove this manually because Apple said it cleans it up for you now. – Lance Samaria Mar 14 '18 at 15:02
  • @Bruno Pinheiro That's probably my best bet to just redo everything with just those 3 vcs or like rmaddy said and use instruments. Thanks! – Lance Samaria Mar 14 '18 at 15:04
  • @LanceSamaria Or you could just quickly check things using the memory graph debugger and see what object are holding references to your secondVC. That would really help and fasten things up when debugging memory problems. – badhanganesh Mar 14 '18 at 15:08
  • @BadhanGanesh Thanks for the help. I’ll look into it – Lance Samaria Mar 14 '18 at 15:09
  • @BadhanGanesh you was correct. As I went through the code and started commented out things I found a 'addPeriodicTimeObserver' that wasn't being released. Once I commented it out I got the deinit to print. I thought I removed it but I didn't. Thanks! – Lance Samaria Mar 14 '18 at 19:42
  • @BrunoPinheiro you was correct it was a strong retain issue. I removed all my KVOs except 1. Thanks for the help! – Lance Samaria Mar 14 '18 at 19:51

2 Answers2

2

The answer to my question is Yes the child View Controller's deinit should also run. If you call deinit inside the child view controller and when the parent is popped of the scene (or dismissed) and the child’s deinit doesn’t run then you have a problem.

As @BadhanGanesh pointed out in the comments he asked:

"are you using any notification observers that you fail to remove them when not needed"

And @Bruno Pinheiro suggested in the comments:

"It seems to be a strong reference issue"

They were both right. I looked through all the code and I thought I had removed a KVO observer but I didn't.

Long story short if you have a View Controller that is a child of another View Controller (parent) then once the parent is deallocated then so should the child.

If your using any KVO observers inside either the parent or child then make sure you remove them or you will create a strong retain cycle.

Lance Samaria
  • 17,576
  • 18
  • 108
  • 256
0

I needed to pass "weak self" to firebase observer in parent view controller to remove the retain cycle, then deinit was called on both parent and child controllers:

    func observeFeatureChanged(){
    microbeRef.queryOrdered(byChild: Nodes.TIMESTAMP)
        .observe(.childChanged) { [weak self] (dataSnapshot) in
            if let featureDic = dataSnapshot.value as? [String: Any]{
                let featureName = dataSnapshot.key
                if let index = self?.featureNames.firstIndex(of: featureName){
                    self?.featureNames[index] = featureName
                    self?.featureDictionaries[index] = featureDic
                    let indexpath = IndexPath(item: index, section: 0)
                    self?.tableView.reloadRows(at: [indexpath], with: .automatic)
                }
            }
    }
}
fullmoon
  • 8,030
  • 5
  • 43
  • 58