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.

Answer 1

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.

Answer 2

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.