fix: tear down iOS NavView on Fabric invalidate so the map releases location

[christian-apollo]

Fixes #631

On iOS (Fabric), unmounting a NavigationView leaks the NavViewController and its GMSMapView: NavViewModule's static registry holds the controller strongly, and the controller holds the NavView back strongly through its _viewCallbacks ivar. The only teardown path is NavView's -dealloc, which that cycle prevents from ever running. The leaked map view stays attached to the navigation session and keeps a kCLLocationAccuracyBestForNavigation CLLocationManager subscribed for the rest of the process — the status-bar location indicator stays on with no map on screen and no guidance running, and with "Always" authorization iOS keeps feeding it in the background.

Changes

• NavView.mm: override -invalidate — the hook Fabric actually calls for unmounted views that are not pooled (shouldBeRecycled is NO) — and tear down from there.
• NavView.mm: cache the nativeID at registration time in a registeredNativeID property, so unregisterView no longer depends on _props still being readable at teardown.
• NavView.mm: in unregisterView, also call [_viewController cleanup] and nil the controller. cleanup already exists and nils _viewCallbacks, which breaks both sides of the cycle so view and controller can deallocate.
• NavViewController.h: declare the existing cleanup method (previously only reachable from the controller's own -dealloc, i.e. never).

Testing

We ship this exact change in production (Apollo Scooters app, RN 0.86, New Architecture) via patch-package:

• Before: mount + unmount a NavigationView, background the app → location indicator stays on; Xcode memory graph shows the NavView ↔ NavViewController cycle with the GMSMapView alive.
• After: backgrounding with the view unmounted leaves no location subscription; controller and map view deallocate on unmount.
• No regressions: the map initializes, renders, re-initializes on re-mount, and guidance keeps location alive mid-navigation as expected.

[yuto-hagatch]

This fix looks solid for the unmount-while-session-alive case, and we can confirm the NavView.mm / unregisterView → cleanup mechanism works (we independently verified the same cleanup call releases _mapView and nils _viewCallbacks correctly).

One gap I think is still open after this PR: it only releases the map view when the view unmounts (-invalidate). If an app instead calls navigationController.cleanup() to tear down the session while keeping the NavigationView mounted for a bit longer (e.g. an exit transition before navigating away), the GMSMapView — and the CLLocationManager subscription this issue is about — keeps running for however long the unmount is delayed, since NavViewModule.navigationSessionDestroyed (called from NavModule's cleanup:) still only does this:

// NavViewModule.mm — unchanged by this PR
- (void)navigationSessionDestroyed {
  for (NavViewController *viewController in [NavViewModule viewControllersRegistry].allValues) {
    [viewController detachFromNavigationSession];
  }
}

detachFromNavigationSession doesn't call cleanup, so the map view and its location subscription outlive the session until the view eventually unmounts and this PR's -invalidate path kicks in.

We hit exactly this in a repro that calls cleanup() then unmounts (mount → guidance → cleanup() → unmount, repeated): native memory grew on every cycle even before we knew about this PR. With this PR alone it should still resolve once the unmount happens, but the release is no longer tied to when the app actually intends to tear the session down.

Would it make sense to also release navigation-type controllers eagerly from navigationSessionDestroyed, using the same cleanup this PR exposes? We tried this in a minimal repro app, gated to isNavigationView controllers so a co-mounted MapView isn't affected by an unrelated session teardown, and it resolves the same open/close memory growth:

// NavViewController.mm — track whether this is a navigation view (vs. a map-only view)
- (void)setMapViewType:(MapViewType)mapViewType {
  ...
  *_mapViewType = mapViewType;
  self.isNavigationView = (mapViewType == NAVIGATION);
}

// NavViewModule.mm
- (void)navigationSessionDestroyed {
  NSMutableDictionary<NSString *, NavViewController *> *registry =
      [NavViewModule viewControllersRegistry];

  for (NSString *nativeID in [registry.allKeys copy]) {
    NavViewController *viewController = registry[nativeID];
    if (viewController.isNavigationView) {
      // Release immediately instead of waiting for the (possibly delayed) unmount.
      [viewController cleanup];
      [registry removeObjectForKey:nativeID];
    } else {
      [viewController detachFromNavigationSession];
    }
  }
}

[illuminati1911]

@christian-apollo Thank you for your contribution. Could you please test the example app that is bundled with the SDK and follow the reproduction flow that you shared?

Before: mount + unmount a NavigationView, background the app → location indicator stays on; Xcode memory graph shows the NavView ↔ NavViewController cycle with the GMSMapView alive.
After: backgrounding with the view unmounted leaves no location subscription; controller and map view deallocate on unmount.
No regressions: the map initializes, renders, re-initializes on re-mount, and guidance keeps location alive mid-navigation as expected.

When I tested this, the location still stays active on the status bar even after leaving the navigation page and backgrounding the application. Do you get the same result?
Edit: This is actually just the example app retaining the navigation session independently from the navigation page unless it's explicitly disposed with "clean up" button.

That being said I was able to see the memory leak before and after your fix it's gone. 👍

[illuminati1911]

This looks good to me and it solves the issue. Please remove all the comments from the code though as they are overly verbose and the code is mostly self-explanatory as well.

[illuminati1911]

This fix looks solid for the unmount-while-session-alive case, and we can confirm the NavView.mm / unregisterView → cleanup mechanism works (we independently verified the same cleanup call releases _mapView and nils _viewCallbacks correctly).

One gap I think is still open after this PR: it only releases the map view when the view unmounts (-invalidate). If an app instead calls navigationController.cleanup() to tear down the session while keeping the NavigationView mounted for a bit longer (e.g. an exit transition before navigating away), the GMSMapView — and the CLLocationManager subscription this issue is about — keeps running for however long the unmount is delayed, since NavViewModule.navigationSessionDestroyed (called from NavModule's cleanup:) still only does this:

// NavViewModule.mm — unchanged by this PR
- (void)navigationSessionDestroyed {
  for (NavViewController *viewController in [NavViewModule viewControllersRegistry].allValues) {
    [viewController detachFromNavigationSession];
  }
}

detachFromNavigationSession doesn't call cleanup, so the map view and its location subscription outlive the session until the view eventually unmounts and this PR's -invalidate path kicks in.

We hit exactly this in a repro that calls cleanup() then unmounts (mount → guidance → cleanup() → unmount, repeated): native memory grew on every cycle even before we knew about this PR. With this PR alone it should still resolve once the unmount happens, but the release is no longer tied to when the app actually intends to tear the session down.

Would it make sense to also release navigation-type controllers eagerly from navigationSessionDestroyed, using the same cleanup this PR exposes? We tried this in a minimal repro app, gated to isNavigationView controllers so a co-mounted MapView isn't affected by an unrelated session teardown, and it resolves the same open/close memory growth:

// NavViewController.mm — track whether this is a navigation view (vs. a map-only view)
- (void)setMapViewType:(MapViewType)mapViewType {
  ...
  *_mapViewType = mapViewType;
  self.isNavigationView = (mapViewType == NAVIGATION);
}

// NavViewModule.mm
- (void)navigationSessionDestroyed {
  NSMutableDictionary<NSString *, NavViewController *> *registry =
      [NavViewModule viewControllersRegistry];

  for (NSString *nativeID in [registry.allKeys copy]) {
    NavViewController *viewController = registry[nativeID];
    if (viewController.isNavigationView) {
      // Release immediately instead of waiting for the (possibly delayed) unmount.
      [viewController cleanup];
      [registry removeObjectForKey:nativeID];
    } else {
      [viewController detachFromNavigationSession];
    }
  }
}

Hey @yuto-hagatch. The SDK is designed so that the navigation session and map view operate independently of each other. They can work together, but closing the map does not imply an intention to end the navigation session, and ending the navigation session does not imply an intention to close the map, even if the map was previously used for navigation.

[christian-apollo]

@illuminati1911 Comments removed and pushed in 2f4fffd. The diff is now just the -invalidate override, the cached registeredNativeID, and the cleanup call in unregisterView.

Thanks for confirming the leak is gone on the example app — and good catch that the example holds the navigation session independently of the page, which is what kept the status-bar indicator on until the explicit "clean up".

[christian-apollo]

@yuto-hagatch Thanks for digging into this, and for independently verifying the cleanup path.

On the navigationSessionDestroyed proposal, I'd side with @illuminati1911 on scope: the session and the map view are independent in this SDK, so eagerly releasing navigation-type controllers there would tear down a map the app still has mounted and hasn't asked to close. That's a behavior change for every consumer, not a leak fix, so it seems better as its own issue/PR than folded in here.

For the repro you describe — cleanup() then a delayed unmount — this PR does release the map view and its location subscription, just at unmount rather than at cleanup(). If holding it for the length of an exit transition is a problem in practice, unmounting the NavigationView before (or as part of) that transition releases it at the point you intend.