56

If I have stored a cancellable set into a ViewController:

private var bag = Set<AnyCancellable>()

Which contains multiple subscription.

1 - Should I cancel subscription in deinit? or it does the job automatically?

2 - If so, how can I cancel all the stored subscriptions?

bag.removeAll() is enough?

or should I iterate through the set and cancel all subscription one by one?

for sub in bag {
   sub.cancel()
}

Apple says that the subscription is alive until the stored AnyCancellable is in memory. So I guess that deallocating the cancellables with bag.removeAll() should be enough, isn't it?

mfaani
  • 33,269
  • 19
  • 164
  • 293
Andrea Miotto
  • 7,084
  • 8
  • 45
  • 70

5 Answers5

39

On deinit your ViewController will be removed from memory. All of its instance variables will be deallocated.

The docs for Combine > Publisher > assign(to:on:) say:

An AnyCancellable instance. Call cancel() on this instance when you no longer want the publisher to automatically assign the property. Deinitializing this instance will also cancel automatic assignment.

1 - Should I cancel subscription in deinit? or it does the job automatically?

You don't need to, it does the job automatically. When your ViewController gets deallocated, the instance variable bag will also be deallocated. As there is no more reference to your AnyCancellable's, the assignment will end.

2 - If so, how can I cancel all the stored subscriptions?

Not so. But often you might have some subscriptions that you want to start and stop on, say, viewWillAppear/viewDidDissapear, for example. In this case your ViewController is still in memory.

So, in viewDidDissappear, you can do bag.removeAll() as you suspected. This will remove the references and stop the assigning.

Here is some code you can run to see .removeAll() in action:

var bag = Set<AnyCancellable>()

func testRemoveAll() {
  Timer.publish(every: 1, on: .main, in: .common).autoconnect()
    .sink { print("===== timer: \($0)") }
    .store(in: &bag)

  Timer.publish(every: 10, on: .main, in: .common).autoconnect()
    .sink { _ in self.bag.removeAll() }
    .store(in: &bag)
}

The first timer will fire every one second and print out a line. The second timer will fire after 10 seconds and then call bag.removeAll(). Then both timer publishers will be stopped.

https://developer.apple.com/documentation/combine/publisher/3235801-assign

Andy
  • 4,441
  • 1
  • 19
  • 16
23

if you happened to subscribe to a publisher from your View controller, likely you will capture self in sink, which will make a reference to it, and won't let ARC remove your view controller later if the subscriber didn't finish, so it's, advisable to weakly capture self


so instead of:

   ["title"]
      .publisher
      .sink { (publishedValue) in
        self.title.text = publishedValue
    }

.store(in: &cancellable)

you should use a [weak self]:

   ["title"]
      .publisher
      .sink { [weak self] (publishedValue) in
        self?.title.text = publishedValue
    }

.store(in: &cancellable)

thus, when View controller is removed, you won't have any retain cycle or memory leaks.

bkbeachlabs
  • 2,121
  • 1
  • 22
  • 33
Mostfa Essam
  • 745
  • 8
  • 11
  • 9
    how does this answers his question though? – giorashc Feb 22 '21 at 17:06
  • 14
    @giorashc user asked if he should cancel subscriptions manually or "it does the job automatically", my answer shows how you can avoid thinking about subscriptions if there's no any strong references to self. by using this way, subscriptions will be removed automatically. – Mostfa Essam Feb 23 '21 at 12:24
12

Try creating a pipeline and not storing the cancellable in some state variable. You’ll find that the pipeline stops as soon as it encounters an async operation. That’s because the Cancellable was cleaned up by ARC and it was thus automatically cancelled. So you don’t need to call cancel on a pipeline if you release all references to it.

From the documentation:

An AnyCancellable instance automatically calls cancel() when deinitialized.

Gil Birman
  • 35,242
  • 14
  • 75
  • 119
  • 1
    it does not seem to work this way as documentation say. I tested syncRequest().sink().store(in: &disposables) and defined it on viewmodel and add deinit { } to view model. deinit prints each time switching screens but subscription receiveCancel is not call and receiveValue is called multiple times – Michał Ziobro May 19 '20 at 10:41
  • 1
    @MichałZiobro sounds like a good question for stackoverflow :D – Gil Birman May 19 '20 at 18:13
  • `subscriptions.removeAll()` works fine in Swift 5.4 – Mark Jun 01 '21 at 15:45
9

I test this code

let cancellable = Set<AnyCancellable>()

Timer.publish(every: 1, on: .main, in: .common).autoconnect()
  .sink { print("===== timer: \($0)") }
  .store(in: &cancellable)
  
cancellable.removeAll() // just remove from Set. not cancellable.cancel()

so I use this extension.

import Combine

typealias CancelBag = Set<AnyCancellable>

extension CancelBag {
  mutating func cancelAll() {
    forEach { $0.cancel() }
    removeAll()
  }
}
Hun
  • 3,652
  • 35
  • 72
2

Create a Cancellable+Extensions.swift

import Combine

typealias DisposeBag = Set<AnyCancellable>

extension DisposeBag {
    mutating func dispose() {
        forEach { $0.cancel() }
        removeAll()
    }
}

In your implementation class, in my case CurrentWeatherViewModel.swift simply add disposables.dispose() to remove Set of AnyCancellable

import Combine
import Foundation

final class CurrentWeatherViewModel: ObservableObject {
    @Published private(set) var dataSource: CurrentWeatherDTO?

    let city: String
    private let weatherFetcher: WeatherFetchable
    private var disposables = Set<AnyCancellable>()

    init(city: String, weatherFetcher: WeatherFetchable = WeatherNetworking()) {
        self.weatherFetcher = weatherFetcher
        self.city = city
    }

    func refresh() {
        disposables.dispose()

        weatherFetcher
            .currentWeatherForecast(forCity: city)
            .map(CurrentWeatherDTO.init)
            .receive(on: DispatchQueue.main)
            .sink(receiveCompletion: { [weak self] value in
                guard let self = self else { return }
                switch value {
                case .failure:
                    self.dataSource = nil
                case .finished:
                    break
                }
                }, receiveValue: { [weak self] weather in
                    guard let self = self else { return }
                    self.dataSource = weather
            })
            .store(in: &disposables)
    }
}
Nischal Hada
  • 3,230
  • 3
  • 27
  • 57
  • 3
    Why do you explicitly call `cancel` for each `AnyCancelable` in the set? Just calling `removeAll()` is enough to set them nil and cancel the ongoing subscriber tasks. – Parth May 21 '21 at 14:43
  • You shouldn't use publishers like this and manually dispose it. You should make use of a publisher and then put your `weatherFetcher` in a `flatMap` and put it in your `init` – Henry Ngan Apr 11 '22 at 15:02
  • can't import combine . its not in scope ? – Neo42 Sep 21 '22 at 17:22
  • Are you suggesting creating a different dispose bag for an additional subscription as you are cancelling on refresh? – dip Jun 01 '23 at 06:27