FlowStacks icon indicating copy to clipboard operation
FlowStacks copied to clipboard

EnvironmentObject FlowNavigator causes memory leak

Open Brudus opened this issue 2 years ago • 1 comments

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

  • Operating system: iOS 17.2
  • Device: iPhone 14 Pro
  • FlowStacks version: 0.3.8
  • How is FlowStacks embedded: SPM

Let me know, if you need more information. Thanks for the great work with FlowStacks!

Brudus avatar Jan 22 '24 14:01 Brudus

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:

allanmaral avatar Feb 02 '24 01:02 allanmaral

The issue is indeed fixed with a newer version. The remaining problems where caused by our code and additionally another third-party-library that had a retain cycle.

Brudus avatar Mar 18 '25 10:03 Brudus

I think this may still be an issue and the NavigationView/navigationBarHidden(true) work around introduces other issues. Curious if others are seeing the same thing.

Heres a small sample app to reproduce it. If you push on many views then go back to the root and use the memory debug tools you'll see many Foo objects still in memory and there should only be 1 for the root view. I've also tracked it to the FlowNavigator and just removing it from the environment fixes it. Really not sure what to do since it seems like it is indeed due to an Apple bug.

@Observable
class Foo: Hashable {
    static func == (lhs: Foo, rhs: Foo) -> Bool { lhs === rhs }
    func hash(into hasher: inout Hasher) {
        hasher.combine(ObjectIdentifier(self))
    }
}

struct FooView: View {
    @State var foo: Foo
    let action: () -> Void
    
    var body: some View {
        Button("Push", action: action)
    }
}

@Observable
class Coordinator {
    let root = Foo()
    var routes: [Route<Foo>] = []
}

struct ContentView: View {
    @State var coordinator: Coordinator
    
    var body: some View {
        FlowStack($coordinator.routes, withNavigation: true) {
            FooView(foo: coordinator.root) {
                coordinator.routes.push(Foo())
            }
            .flowDestination(for: Foo.self) { foo in
                FooView(foo: foo) {
                    coordinator.routes.push(Foo())
                }
            }
        }
    }
}

wickwirew avatar Apr 25 '25 17:04 wickwirew