SwiftUI ViewModel not being deinit and causing memory leak

I am developing a Mac OS app that pulls football standings from an API and displays it in a View. I noticed that my memory keeps increasing and after using the Xcode Memory Graph, I believe that my problem is that the ViewModel is not being deinit and thus still being allocated memory. For example, in the picture below, going back and forth from the BundesligaView 3 times has created 3 separate models.

Memory Graph

// VIEW MODEL
class BundesligaViewModel: ObservableObject {
    @Published var standings: [BundesligaModel] = []
    @Published var loadingState: LoadingState = .idle
    
    @MainActor
    func fetchData(_ league: String? = nil) async {
        loadingState = .loading
        do { 
            standings = try await NetworkManager.shared.fetchBundesligaStandings(league!)
            loadingState = .loaded
        } catch {
            print("Error fetching standings: \(error)")
            loadingState = .error
        }
    }
    
    init() {
        print("VIEWMODEL CREATED")
    }

    deinit {
        print("VIEWMODEL DEINT")
    }
}

enum LoadingState {
    case idle
    case loading
    case loaded
    case error
}
// VIEW
struct LeagueIndex: View {
    var body: some View {
        NavigationStack {
            NavigationLink(destination: BundesligaTable(league: "Bundesliga")) {
                Text("Bundesliga")
            }
            .buttonStyle(PlainButtonStyle())
        }
        .navigationBarBackButtonHidden(true)
    }
}

struct Footer: View {
    var body: some View {
        NavigationLink(destination: LeagueIndex()) {
            Text("League Index").foregroundStyle(Color.red)
        }
        .buttonStyle(PlainButtonStyle())
    }
}

struct BundesligaTable: View {
    let league: String
    @StateObject private var viewModel = BundesligaViewModel()
    
    let columns = [
        GridItem(.fixed(25)),
        GridItem(.flexible()),
        GridItem(.fixed(25)),
        GridItem(.fixed(25)),
        GridItem(.fixed(25)),
        GridItem(.fixed(25)),
        GridItem(.fixed(25)),
        GridItem(.fixed(25)),
        GridItem(.fixed(25)),
        GridItem(.fixed(25))
    ]
    
    var body: some View {
        ScrollView {
            LazyVGrid(columns: columns, spacing: 10) {
                ForEach(Array(viewModel.standings.enumerated()), id: \.element.teamName) { index, team in
                    Text("\(index + 1)")
                    Text(team.shortName)
                    Text("\(team.matches)")
                    Text("\(team.won)")
                    Text("\(team.draw)")
                    Text("\(team.lost)")
                    Text("\(team.goals)")
                    Text("\(team.opponentGoals)")
                    Text("\(team.goalDiff)")
                    Text("\(team.points)")
                }
            }
            Footer()
        }
        .padding(.horizontal)
        .task { 
            await viewModel.fetchData(league)
        }
    }
}

Is there any problem with my code? Should i use [weak self] somewhere. Let me know if you need any more context/code for clarity. Thanks for looking into this!

Even though this issue is given in SwiftUI, I have had a similar issue when working with async/await and unexpected holds on objects. Luckily, I have a previous behavior sample I had created and wanted to write about.

If you run the first run block below in a macOS command line project (or on the command line) you'll notice that the ViewModel instance does not get released when expected (mimicking the view with the StateObject being dismissed) due to the fact that an async function on your object is being await-ed. In the Xcode memory graph, the references to your object are those AsyncSlab instances. The second run block is just one example of how to mitigate it and properly get released when expected.

func run(for duration: TimeInterval, execute action: @escaping () async throws -> Void) {
    
    class State {
        let start = Date.now
    }
    
    let s = State()
    
    Task {
        do {
            try await action()
        } catch {
            print("Error: \(error)")
        }
    }
    
    while Date.now.timeIntervalSince(s.start) < duration {
        RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.1))
    }
}

class ViewModel {
    
    var items: [Int] = [] {
        // This should *not* be set after a Task was cancelled
        didSet {
            print("items.didSet")
        }
    }
    
    // Use `AnyCancellable` because it will call `cancel` when deinit-ed.
    private var currentPagingRequest: AnyCancellable? {
        didSet {
            print("did set currentPagingRequest")
        }
    }
    
    deinit {
        print("ViewModel.deinit")
    }
    
    func refresh() {
        currentPagingRequest = Task { [weak self] in
            // Suspension points will create references to the object.
            let items = try await self?.networkCall()
            return items
        }
        .asAnyCancellable()
    }
    
    func outsideRefresh() {
        currentPagingRequest = Task {
            // This will not create a reference to the object
            let items = try await outsideNetworkCall()
            return items
        }
        .asAnyCancellable()
    }
    
    // This is a separate async function because it mimics a real-life
    // scenario where subclasses are supposed to override this function
    // for their context-specific network calls, but the usage of the results
    // is the same.
    func networkCall() async throws -> [Int] {
        
        // This is supposed to mimic a network call that may
        // take a long time and may return a lot of data.
        //
        // Suspension points from this async function and the
        // await below make references to the object (AsyncSlab).
        try! await Task.sleep(for: .seconds(3))
        
        // This returns `false`, since the above suspension point
        // has completed but not the suspension point within the caller
        print("was networkCall cancelled: \(Task.isCancelled)")
        
        return Array(0 ..< 10)
    }
}

// This is an outside function that doesn't live within the object
func outsideNetworkCall() async throws -> [Int] {
    
    // This is supposed to mimic a network call that may
    // take a long time and may return a lot of data.
    //
    // Since this function lives *outside* `ViewModel`, it doesn't
    // reference it and will properly get a cancel error.
    do {
        try await Task.sleep(for: .seconds(3))
    } catch {
        if error is CancellationError {
            print("Got cancellation error!")
        }
    }
    
    return Array(0 ..< 10)
}

// Run to show that something isn't being released
run(for: 7) {
    let viewModel = ViewModel()
    viewModel.refresh()
    
    // Mimic some time for user interaction
    try! await Task.sleep(for: .seconds(1))
    
    // Relese object reference, similar to View
    // releasing via `StateObject`
    let _ = consume viewModel
    
    print("ViewModel will de-init a little longer after this statement")
}

// Run to show that something is being released
//run(for: 7) {
//    let viewModel = ViewModel()
//    viewModel.outsideRefresh()
//    
//    // Mimic some time for user interaction
//    try! await Task.sleep(for: .seconds(1))
//    
//    // Relese object reference, similar to View
//    // releasing via `StateObject`
//    let _ = consume viewModel
//    
//    print("ViewModel should de-init before or right after this statement")
//}

Mitigation

I have done a few things to potentially mitigate the issues:

1 - Wrapping my tasks in AnyCancellable. I do this because I hold onto my tasks to cancel previous tasks and also because when the object is deinit-ed the cancel function it automatically called.
2 - You can potentially use an outside service source and not use async functions on your object or:
3 - Don't call async functions on your object that may be expected to greatly outlive their expected use
4 - Ignore it. Both of our examples are about network calls and this kind of allows us to be flexible.

  • If the network call fails for whatever reason, you'll probably get an exception that is being handled and once that function completes the AsyncSlab references will be released.
  • If the network call succeeds, all you did was keep in memory the data from the network call a little bit longer which again, will get released shortly after the function returns.

There isn't a "right way" to go about this, however you should strongly consider how your views and view models are being used by the user. If this view is being shown and dismissed within a short period of time often, you may want to release the memory as soon as you can, refactor so you don't have to make an outside network call so often, or redesign in such a way that this view/data is being fetched and dismissed so often.

1 Like

That's my guess too. And I suspect that once the async func returns or times out, the instance should be destroyed? If so, I wonder why it's an issue?

The async tasks created with the task view modifier has the same lifecycle as the view: it cancels the task when the view disappears. Sometimes a SwiftUI view isn’t disappeared even though you can’t see it (e.g., TabView child views). If you add an onDisappear action, does it run when you’d expect your model to deinit?

1 Like

Thanks @adamkuipers for pointing me in the right direction! You were right, my View was indeed not disappearing. After much more investigations, I realised that I had a custom Footer which navigated to LeagueIndex when in BundesligaView. It was supposed to act as a back button in the NavigationStack, however it was wrongly implemented (creating new iterations of LeagueIndex rather than going back) causing both the View and the ViewModel to retain.

Old Footer:

struct Footer: View {
    var body: some View {
        NavigationLink(destination: LeagueIndex()) {
            Text("League Index").foregroundStyle(Color.red)
        }
    }
}

New Footer:

struct Footer: View {
    @Environment(\.dismiss) var dismiss
    
    var body: some View {
          Button(action: { dismiss() }) {
              HStack {
                  Text("League Index").foregroundStyle(Color.red)
              }
         }
    }
}