Weird retain cycle when using the Coordinator pattern

Question

I am building a new app using MVVM + Coordinators. Specifically, I am using the Coordinator pattern found on https://github.com/daveneff/Coordinator.

On the top level I have an AppCoordinator that can start the child coordinator RegisterCoordinator. When the sign up flow is complete, the AppCoordinator then switches the root viewcontroller of its navigator, and the coordinators and viewcontrollers used in the sign up flow should be released from memory.

final class AppCoordinator: CoordinatorNavigable {
  var dependencies: AppDependencies
  var childCoordinators: [Coordinator] = []
  var rootViewController = UINavigationController()
  var navigator: NavigatorType

  init(window: UIWindow, dependencies: AppDependencies) {
    self.dependencies = dependencies
    navigator = Navigator(navigationController: rootViewController)

    dependencies.userManager.delegate = self

    window.rootViewController = rootViewController
    window.makeKeyAndVisible()
  }

  func start() {
    if dependencies.properties[.user] == nil {
      // Logged out state
      let vc = AuthFlowViewController.instantiate(storyboardName: Constants.Storyboards.authFlow)
      vc.delegate = self
      navigator.setRootViewController(vc, animated: false)
    } else {
      // Logged in
      let vc = HomeViewController.instantiate(storyboardName: Constants.Storyboards.home)
      vc.viewModel = HomeViewModel(dependencies: dependencies)
      navigator.setRootViewController(vc, animated: false)
    }

    childCoordinators = []
  }
}

extension AppCoordinator: UserManagerDelegate {
  func authStateChanged() {
    // User logged in or logged out; show the correct root view controller
    start()
  }

  func userChanged() {}
}

extension AppCoordinator: AuthFlowViewControllerDelegate {
  func login() {
    dependencies.userManager.changeUser(newUser: User(id: 1, name: "Kevin"))
  }

  func startRegisterFlow() {
    let registerCoordinator = RegisterCoordinator(dependencies: dependencies, navigator: navigator)
    pushCoordinator(registerCoordinator, animated: true)
  }
}

The RegisterCoordinator meanwhile simply pushes multiple viewcontrollers onto the navigator's stack:

class RegisterCoordinator: CoordinatorNavigable {
  var dependencies: AppDependencies
  var childCoordinators: [Coordinator] = []
  var navigator: NavigatorType

  let rootViewController = PhoneInputViewController.instantiate(storyboardName: Constants.Storyboards.authFlow)

  init(dependencies: AppDependencies, navigator: NavigatorType) {
    self.dependencies = dependencies
    self.navigator = navigator
    rootViewController.delegate = self
  }

  func start() {}
}

extension RegisterCoordinator: PhoneInputViewControllerDelegate {
  func phoneInputDone() {
    let vc = PhoneValidationViewController.instantiate(storyboardName: Constants.Storyboards.authFlow)
    vc.delegate = self
    navigator.push(vc, animated: true)
  }
}

extension RegisterCoordinator: PhoneValidationViewControllerDelegate {
  func phoneValidationDone() {
    let vc = GenderSelectionViewController.instantiate(storyboardName: Constants.Storyboards.authFlow)
    vc.viewModel = GenderSelectionViewModel(dependencies: dependencies)
    navigator.push(vc, animated: true)
  }
}

When the entire sign up flow is complete, the last page can save the user, which triggers the authStateChanged method in the AppCoordinator, which then changes the navigator's rootViewController. This should then clean up its child coordinators as well.

Sadly though, the RegisterCoordinator and its rootViewController (PhoneInputViewController) are kept alive - although the other viewcontrollers in the flow are properly released.

I tried to manually do childCoordinators = [] in the start method to make sure the AppCoordinator doesn't have a strong reference to the RegisterCoordinator, but even that doesn't help.

I have no clue what is keeping the strong reference, causing the retain cycle / memory leak. I have a super minimal version of my app with basically everything removed except the bare essentials to show the problem, up on GitHub: https://github.com/kevinrenskers/coordinator-problem.

Also please don't post example code with pods in it. I would be happy to download your project and run it, but not with a pod in it. — matt, Jul 19 '19 at 13:02
There is one pod, it's already committed, you don't have to do anything yourself. I don't see the problem? — Kevin Renskers, Jul 19 '19 at 13:07
Not sure what you mean by that. Your project won't compile because Defaults module is missing. That's the pod. I'm not installing it just to run your project. — matt, Jul 19 '19 at 13:09
Ah sorry, Skippit.xcworkspace was not correctly committed. Now it is, so you don't have to install anything. — Kevin Renskers, Jul 19 '19 at 13:14
Nope, still can't compile it. I really would like to show you how you could have just found this leak diagrammed for you in the object graph or Instruments but it isn't going to happen, sorry. — matt, Jul 19 '19 at 14:06
I did a fresh clone of the GitHub repo and it works totally fine here, without having to install anything. Just make sure you open the workspace, not the project. But thanks anyway, I appreciate the effort! — Kevin Renskers, Jul 19 '19 at 14:19

rkyr · Answer 1 · 2019-07-19T13:03:24.280

4

First of all, you're capturing your coordinator inside a block in Coordinator.self line 132:

I found this using Debug Memory Graph:

there's also PhoneInputViewController still alive, you can examine why using the same method

I can't fully understand how your coordinator pattern implementation work, but it's a good idea not to keep strong refs to your controllers.

I've been using some implementation, where controllers being kept only by UINavigationController's stack, and window keeps UINavigationController.

It guarantees that your controllers will always die once popped/replaced.

In your case, I would start by trying making childCoordinators of Coordinator to keep weak refs to your controllers.

edited Jul 19 '19 at 13:03

answered Jul 19 '19 at 12:50

rkyr

3,131
2
23
38

I think the real reason is line 94 in the Navigator: `completions[viewController] = completion`. If I just remove that, the retain cycle is gone when you complete the whole sign up flow. However, if you simply start the sign up flow and then press the back button, then the retain cycle is there - so the problem simply moved. Not sure why the `completions` dictionary is not properly cleaned up after setting the root VC. Looking into that now. – Kevin Renskers Jul 19 '19 at 13:29
Aha! Adding `completions = [:]` after line 101 in the Navigator solves the problem. Sent a PR to the creator of this Coordinator: https://github.com/daveneff/Coordinator/pull/1. – Kevin Renskers Jul 19 '19 at 13:32
@KevinRenskers I have a similar question for the correct practecies. Not using a pod but trying to pop back to rootVC when UI reference lost. https://stackoverflow.com/questions/59486627/back-button-not-being-called-in-tabbarcoordinator-in-horizontal-flow-ios-12 – Marina Dec 26 '19 at 15:36
Very well explained. – Marcel Alves Sep 24 '20 at 14:23

score 0 · Answer 2 · answered Jul 19 '19 at 13:43

The answer from rkyr pushed me in the right direction, and I found the source of the problem and sent a PR with the fix to the original Coordinator library that I am using. So check it out there for the one-line fix: https://github.com/daveneff/Coordinator/pull/1.

Weird retain cycle when using the Coordinator pattern

2 Answers2