How can I avoid this SwiftUI + Combine Timer Publisher reference cycle / memory leak?

Viewed 1014

I have the following SwiftUI view which contains a subview that fades away after five seconds. The fade is triggered by receiving the result of a Combine TimePublisher, but changing the value of showRedView in the sink publisher's sink block is causing a memory leak.

import Combine
import SwiftUI

struct ContentView: View {
    @State var showRedView = true

    @State var subscriptions: Set<AnyCancellable> = []
    
    var body: some View {
        ZStack {
            if showRedView {
                Color.red
                    .transition(.opacity)
            }
            Text("Hello, world!")
                .padding()
        }
        .onAppear {
            fadeRedView()
        }
    }
    
    func fadeRedView() {
        Timer.publish(every: 5.0, on: .main, in: .default)
            .autoconnect()
            .prefix(1)
            .sink { _ in
                withAnimation {
                    showRedView = false
                }
            }
            .store(in: &subscriptions)
    }
}

I thought this was somehow managed behind the scenes with the AnyCancellable collection. I'm relatively new to SwiftUI and Combine, so sure I'm either messing something up here or not thinking about it correctly. What's the best way to avoid this leak?

Edit: Adding some pictures showing the leak.

Memory leak pic 1

Memory leak pic 2

2 Answers

Views should be thought of as describing the structure of the view, and how it reacts to data. They ought to be small, single-purpose, easy-to-init structures. They shouldn't hold instances with their own life-cycles (like keeping publisher subscriptions) - those belong to the view model.

class ViewModel: ObservableObject {
   var pub: AnyPublisher<Void, Never> {
        Timer.publish(every: 2.0, on: .main, in: .default).autoconnect()
            .prefix(1)
            .map { _ in }
            .eraseToAnyPublisher()
    } 
}

And use .onReceive to react to published events in the View:

struct ContentView: View {
    @State var showRedView = true

    @ObservedObject vm = ViewModel()
    
    var body: some View {
        ZStack {
            if showRedView {
                Color.red
                    .transition(.opacity)
            }
            Text("Hello, world!")
                .padding()
        }
        .onReceive(self.vm.pub, perform: {
            withAnimation {
                self.showRedView = false
            }
        })
    }
}

So, it seems that with the above arrangement, the TimerPublisher with prefix publisher chain is causing the leak. It's also not the right publisher to use for your use case.

The following achieves the same result, without the leak:

class ViewModel: ObservableObject {
   var pub: AnyPublisher<Void, Never> {
        Just(())
           .delay(for: .seconds(2.0), scheduler: DispatchQueue.main)
           .eraseToAnyPublisher()
    } 
}

My guess is that you're leaking because you store an AnyCancellable in subscriptions and you never remove it.

The sink operator creates the AnyCancellable. Unless you store it somewhere, the subscription will be cancelled prematurely. But if we use the Subscribers.Sink subscriber directly, instead of using the sink operator, there will be no AnyCancellable for us to manage.

    func fadeRedView() {
        Timer.publish(every: 5.0, on: .main, in: .default)
            .autoconnect()
            .prefix(1)
            .subscribe(Subscribers.Sink(
                receiveCompletion: { _ in },
                receiveValue: { _ in
                    withAnimation {
                        showRedView = false
                    }
                }
            ))
    }

But this is still overkill. You don't need Combine for this. You can schedule the event directly:

    func fadeRedView() {
        DispatchQueue.main.asyncAfter(deadline: .now() + .seconds(5)) {
            withAnimation {
                showRedView = false
            }

        }
    }
Related