johnpatrickmorgan / FlowStacks

FlowStacks allows you to hoist SwiftUI navigation and presentation state into a Coordinator
MIT License
856 stars 66 forks source link

EnvironmentObject FlowNavigator causes memory leak #70

Open Brudus opened 10 months ago

Brudus commented 10 months ago

Intro

Hello, our team faces an issue with FlowStacks that causes the app to keep instances of viewModels around even though they are not needed anymore. After a longer debugging session, I've noticed that commenting out one line in FlowStacks resolves the issue.

The problem

Whenever, we 'restart' our process/app (e.g. switching the country), at least the first instance of the viewModel is retained. There are no strong references to it anymore besides from FlowNavigator. This causes our viewModel's subscriptions to still remain active and trigger when data changes causing an increasing number of requests, etc.

How to reproduce this?

I added a small project showcasing this issue - https://github.com/Brudus/FlowStacksMemoryLeak/tree/main

You can either investigate the logs or use the memory graph debugger. As soon as you pressed the reset button once, there should only be a single instance of TabBarCoordinatorViewModel but there are two. At least the very first instance is kept around indefinitely.

I assume the issue was introduced with v0.3.2 when the FlowNavigator was added. The issue can be fixed by commenting out line 35 of Router.swift (.environmentObject(FlowNavigator($routes))), but this is, obviously, no solution for FlowStacks itself.

Environment

allanmaral commented 9 months ago

This problem only seems to happen with embedInNavigationView = true. I believe it is related to some memory leak problems using .environmentObject(_:) with NavigationViews.

A weird hack/workaround would be to embed the root coordinator in a NavigationView with .navigationBarHidden(true) to avoid the title being shown. I'm not sure if this would work for all use cases, though.

import SwiftUI

@main
struct FlowStacksMemoryLeakApp: App {

    @StateObject private var mainCoordinatorViewModel = MainCoordinatorViewModel()

    var body: some Scene {
        WindowGroup {
++            NavigationView {
                MainCoordinatorView(viewModel: mainCoordinatorViewModel)
++                    .navigationBarHidden(true)
++            }
        }
    }
}

See: