fix(ios): hold navigation views weakly in the view registry so they deallocate on teardown - #752
fix(ios): hold navigation views weakly in the view registry so they deallocate on teardown#752rafmsou wants to merge 1 commit into
Conversation
`GoogleMapsNavigationViewRegistry` stored views in a strong dictionary but a view removes itself from the registry only in its `deinit` (`unregisterView()`). The strong entry keeps the view's reference count above zero after Flutter tears down the platform view, so `deinit` never runs, the view is never unregistered, and its underlying `GMSMapView` is retained for the lifetime of the process. Flutter assigns a new view id per platform-view creation, so nothing overwrites the stale entry either — one view leaks per view creation. Hold views via a weak wrapper so a released view deallocates, which triggers `deinit` and prunes the (now-empty) entry. `unregisterView` also drops entries whose weak reference has already been reclaimed so stale keys can't accumulate. CarPlay handling is unchanged (it has an explicit unregister path and never depended on `deinit`). Measured on a physical device: physical footprint climbed ~85 MB -> ~595 MB over ~4.5 min of map open/close cycles before the change, and reclaims between cycles after it.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
| /// platform view, nothing overwrites the stale entry either, so one view leaks | ||
| /// per view creation. Holding views weakly lets a released view deallocate, | ||
| /// which triggers `deinit` and prunes the (now-empty) entry. | ||
| private class WeakViewRef { |
There was a problem hiding this comment.
This is good, but let's make it generic for the whole library. Remove this and add generic version to Utilities.swift.
final class WeakRef<T: AnyObject> {
weak var value: T?
init(_ value: T) {
self.value = value
}
}Also no need to explain all this. Just remove the whole comment.
|
|
||
| class GoogleMapsNavigationViewRegistry { | ||
| private var views: [Int64: GoogleMapsNavigationView] = [:] | ||
| private var views: [Int64: WeakViewRef] = [:] |
There was a problem hiding this comment.
Let's change the name of the dictionary to viewRefs to match the content more accurately.
illuminati1911
left a comment
There was a problem hiding this comment.
Thanks a lot for the contribution. Few changes and I think should be good to go.
Summary
On iOS, every
GoogleMapsMapView/GoogleMapsNavigationViewis retained for the lifetime of the process, so repeatedly creating and disposing map/nav views grows memory without bound and eventually leads to jetsam/WatchdogTerminationon memory-constrained devices.GoogleMapsNavigationViewRegistrystored views in a strong dictionary:but a view removes itself from the registry only in its
deinit(unregisterView()). The strong entry keeps the view's reference count above zero after Flutter tears down the platform view, sodeinitnever runs,unregisterView()is never called, and the view (and its underlyingGMSMapView) leaks forever. Flutter assigns a newviewIdper platform-view creation, so nothing overwrites the stale entry either — one view leaks per view creation.This is a self-referential retain: the only code that removes a view from the registry runs in
deinit, which the registry's own strong reference prevents from ever running.Fix
Hold views through a small
WeakViewRefwrapper instead of strongly. A released view then deallocates,deinitruns, and the entry is pruned.unregisterViewalso removes entries whose weak reference has already been reclaimed, so stale keys can't accumulate. All read accessorscompactMapover live views.CarPlay handling is unchanged:
carPlayViewhas an explicitunregisterCarPlayView()call path (fromBaseCarSceneDelegate) and never depended ondeinit, so it was not affected by this leak.Reproduction / verification
GoogleMapsMapView, dismiss it, repeat.Measured on an iPhone 11 / iOS 26.5: physical footprint climbed ~85 MB → ~595 MB over ~4.5 min of open/close cycles with no reclamation (~88 MB/min) before the change; after the change it reclaims between cycles.
Present at least through 0.9.4 and 0.10.0 (identical registry code).