17

I'd like all publishers to execute unless explicitly cancelled. I don't mind AnyCancellable going out of scope, however based on docs it automatically calls cancel on deinit which is undesired.

I've tried to use a cancellable bag, but AnyCancelable kept piling up even after the publisher fired a completion.

Should I manage the bag manually? I had impression that store(in: inout Set) was meant to be used for convenience of managing the cancellable instances, however all it does is push AnyCancellable into a set.

var cancelableSet = Set<AnyCancellable>()

func work(value: Int) -> AnyCancellable {
    return Just(value)
        .delay(for: .seconds(1), scheduler: DispatchQueue.global(qos: .default))
        .map { $0 + 1 }
        .sink(receiveValue: { (value) in
            print("Got value: \(value)")
        })
}

work(value: 1337).store(in: &cancelableSet)

DispatchQueue.main.asyncAfter(deadline: .now() + .seconds(5)) {
    print("\(cancelableSet)")
}

What I came up with so far, which works fine but makes me wonder if something is missing in the Combine framework or it was not meant to be used in such fashion:

class DisposeBag {
    private let lock = NSLock()
    private var cancellableSet = Set<AnyCancellable>()

    func store(_ cancellable: AnyCancellable) {
        print("Store cancellable: \(cancellable)")

        lock.lock()
        cancellableSet.insert(cancellable)
        lock.unlock()
    }

    func drop(_ cancellable: AnyCancellable) {
        print("Drop cancellable: \(cancellable)")

        lock.lock()
        cancellableSet.remove(cancellable)
        lock.unlock()
    }
}

extension Publisher {

    @discardableResult func autoDisposableSink(disposeBag: DisposeBag, receiveCompletion: @escaping ((Subscribers.Completion<Self.Failure>) -> Void), receiveValue: @escaping ((Self.Output) -> Void)) -> AnyCancellable {
        var sharedCancellable: AnyCancellable?

        let disposeSubscriber = {
            if let sharedCancellable = sharedCancellable {
                disposeBag.drop(sharedCancellable)
            }
        }

        let cancellable = handleEvents(receiveCancel: {
            disposeSubscriber()
        }).sink(receiveCompletion: { (completion) in
            receiveCompletion(completion)

            disposeSubscriber()
        }, receiveValue: receiveValue)

        sharedCancellable = cancellable
        disposeBag.store(cancellable)

        return cancellable
    }

}
pronebird
  • 12,068
  • 5
  • 54
  • 82
  • I don't understand what this has to do with AnyCancellable, bags, publishers, or anything else. To prevent something from being destroyed on `deinit`, keep it somewhere that won't get `deinit`: a global, or a property of your app delegate or root view controller. As the answer says, your problem is merely that you've stored this object in the wrong scope. – matt Nov 05 '19 at 15:15
  • Well you would remove them when done with them. Again I don’t see why this is a special case. – matt Nov 05 '19 at 15:35
  • 2
    @matt not ergonomic. I can define a local `var cancellableSet = Set()` and then reference it in `handleEvents` and call `cancellableSet.removeAll()` for both cancel and completion events. Easy Right? Well not really. 1. `Set` is not thread safe and causes a crash `Simultaneous accesses to XXXX, but modification requires exclusive access.`. 2. It's a lot of boilerplate that can be avoided 3. It's error prone, easy to make a mistake. Again `AnyCancellable.store(in: &cancellableSet)` is by no means a reasonable solution. Something is missing. – pronebird Nov 05 '19 at 16:01
  • Well I suppose you could just let them pile up. This is a tiny object; you could have thousands of them without caring. By the way "Simultaneous accesses" has nothing to do with threading. – matt Nov 05 '19 at 16:28
  • @BenSinclair Have you been able to get any more clarity on this issue? I have the same problem. Making API calls using a publisher, and every call requires storing the sub in cancellable set. They just keep piling up. – Kon Jan 20 '20 at 01:13
  • @Kon not really. It's by design that you have to hold cancellable unless you wish to cancel the stream. This works well in most cases with a few exceptions. If you wish to auto-drop cancellables upon completion, try the extension I wrote (see above). – pronebird Jan 20 '20 at 14:38
  • Hi! Did you use this code in production? How does it perform if that's the case? Thanks! – Objectif Jun 18 '21 at 15:33
  • @Objectif I haven't used it in production. The solution is odd in my opinion. – pronebird Jun 19 '21 at 19:13

1 Answers1

2

The subscriptions in Apple Combine are scoped in a RAII compliant fashion. I.e. the event of deinitialization is equivalent to the event of automatic disposal of the observable. That is contrary to RxSwift Disposable where this behavior is sometimes reproduced, but not strictly so.

Even in RxSwift if you lose a DisposeBag your subscriptions will be disposed and this is a feature. If you would like your subscription to live through the scope, it means that it belongs to an outer scope.

And none of these implementations get busy actually tossing out the Disposables out of the retention tree once the subscriptions are done.

Isaaс Weisberg
  • 2,296
  • 2
  • 12
  • 28
  • Fine by me if that's the case. It seems to me though that the `DisposeBag` is actually missing in Combine, so consumers are forced to maintain the subscribers manually. It's fine when it's just a few subscribers like in UIs, but I have infinite flows like server response handling kind of stuff. Messages arrive, I do some work and fill up the `Set` with `AnyCancellable`. I'd expect to have capabilities to store and auto-evict cancellable from the given Set. – pronebird Nov 05 '19 at 12:51
  • @BenSinclair excuse me if I am wrong and probably Mr. Mishali will be a better source to provide this information, but last time I checked, the DisposeBag actually doesn't ugh... do really anything except providing the synchronization of subscriptions storage. It's not like it releases the cancelable object itself once the subscription is done, it really just stays there and hence everything that this disposable retains. And they-- don't get thrown out.. ever.. no? – Isaaс Weisberg Nov 05 '19 at 13:18
  • Not sure about the RxSwift implementation. Combine only provides means to put cancellables into a `Set` which it never cleans up, which causes dangling objects. In my implementation, a bag is basically a thread safe storage. If you look at my source code, I leverage `handleEvents` to get notified when the publisher completes, that's when I remove the cancellable from the bag. Releasing the bag itself should on the other side cause all of the contained cancellables to trigger the cancellation. – pronebird Nov 05 '19 at 14:26
  • @BenSinclair hey Ben, but why, again, do you need this? Are you doing this because of an optimization concern of any sorts? – Isaaс Weisberg Nov 05 '19 at 14:52
  • 2
    No optimization. Just have stuff to process and need to retain cancellable objects cuz If I don't, my publishers are being cancelled and nothing literally works. If I store cancellables in array or set, they keep piling up, so I have to remove them at some point and there is no better time then removing the cancellable objects as the corresponding publishers complete. – pronebird Nov 05 '19 at 15:24