liveview-native / liveview-client-swiftui

MIT License
379 stars 39 forks source link

Fix retain cycle in LiveViewCoordinator #1455

Closed nighthawk closed 2 months ago

nighthawk commented 2 months ago

LiveViewCoordinator is initialised from LiveSessionCoordinator, which gets passed in as a parameter to init and then retained by the view coordinator. This causes a retain cycle.

Additionally, this is also retained by the handleEvent as session was used directly in the closure, which then retained it.

This commit fixes both by using a weak reference from the view coordinator to the session coordinator, and accessing the session via the weak self reference.

nighthawk commented 2 months ago

The retain cycle is triggered by LiveSessionCoordinator's init method which has this line:

self.navigationPath = [.init(url: url, coordinator: .init(session: self, url: self.url))]

Note how self is passed there, which causes the retain cycle.

I noticed this when I didn't have my local server running and the LiveView in the app couldn't connect, and when I navigated away from the LiveView in my app and it was no longer in the view hierarchy, the live view session kept trying to reconnect.

nighthawk commented 2 months ago

Seems like that retain cycle hid two other issues here. Since LiveSessionCoordinator will now get deallocated properly, the two unused self usages need to be removed as they can fire in rare cases after it got deallocated, which then crashes.

bcardarella commented 2 months ago

This should go under Fixed in the CHANGELOG.md

nighthawk commented 2 months ago

Sure thing. Change log updated.

bcardarella commented 2 months ago

@nighthawk thank you!