27

I don't quite understand how to properly store subscribers inside a class so that they persist but don't prevent the object from being deinitialized. Here's an example where the object won't deinit:

import UIKit
import Combine

class Test {
    public var name: String = ""

    private var disposeBag: Set<AnyCancellable> = Set()

    deinit {
        print("deinit")
    }

    init(publisher: CurrentValueSubject<String, Never>) {
        publisher.assign(to: \.name, on: self).store(in: &disposeBag)
    }
}

let publisher = CurrentValueSubject<String, Never>("Test")

var test: Test? = Test(publisher: publisher)
test = nil

When I replace the assign with a sink (in which I properly declare [weak self]) it actually does deinit properly (probably because the assign accesses self in a way that causes problems).

How can I prevent strong reference cycles when using .assign for instance?

Thanks

SwiftedMind
  • 3,701
  • 3
  • 29
  • 63
  • 1
    This must be a bug in Combine as this would seem to be a fairly common use case. Work around for now is to use `sink`. – Tylerc230 Nov 07 '19 at 19:19

6 Answers6

29

you can replace .asign(to:) with sink where [weak self] in its closure brake the memory cycle. Try it in Playground to see the difference

final class Bar: ObservableObject {
    @Published var input: String = ""
    @Published var output: String = ""

    private var subscription: AnyCancellable?

    init() {
        subscription = $input
            .filter { $0.count > 0 }
            .map { "\($0) World!" }
            //.assignNoRetain(to: \.output, on: self)
            .sink { [weak self] (value) in
                self?.output = value
        }

    }

    deinit {
        subscription?.cancel()
        print("\(self): \(#function)")
    }
}

// test it!!
var bar: Bar? = Bar()
let foo = bar?.$output.sink { print($0) }
bar?.input = "Hello"
bar?.input = "Goodby,"
bar = nil

it prints

Hello World!
Goodby, World!
__lldb_expr_4.Bar: deinit

so we don't have the memory leak !

finally at forums.swift.org someone make a nice little

extension Publisher where Self.Failure == Never {
    public func assignNoRetain<Root>(to keyPath: ReferenceWritableKeyPath<Root, Self.Output>, on object: Root) -> AnyCancellable where Root: AnyObject {
        sink { [weak object] (value) in
        object?[keyPath: keyPath] = value
    }
  }
}
user3441734
  • 16,722
  • 2
  • 40
  • 59
  • Oh, I haven't seen the edit with the new `assignNoRetain` method. Thanks! That's a nice way of hiding the `sink` – SwiftedMind Apr 05 '20 at 11:21
  • This assignNoRetain extension method really breaks the retain cycle and a good shorthand as well. Cheers :) – umali Feb 11 '21 at 21:29
5

How about:

class Test {
    @Published var name: String = ""

deinit {
    print("deinit")
}

init(publisher: CurrentValueSubject<String, Never>) {
    publisher.assign(to: &$name)
}

}

This version of the assign operation manages memory internally (does not return AnyCancellable), as it dies together with the object. Note you need to convert your property to @Published.

gerbil
  • 859
  • 7
  • 26
2

I don't know what you have against closures but the solution is to not use self in the assign:

import Combine
import SwiftUI

class NameStore {
    var name: String
    init() { name = "" }
    deinit { print("deinit NameStore") }
}

class Test {
    private var nameStore = NameStore()
    public var name: String { get { return nameStore.name } }

    var subscriber: AnyCancellable? = nil

    deinit { print("deinit Test") }

    init(publisher: CurrentValueSubject<String, Never>) {
        subscriber = publisher.print().assign(to: \NameStore.name, on: nameStore)
    }
}

let publisher = CurrentValueSubject<String, Never>("Test")
var test: Test? = Test(publisher: publisher)

struct ContentView : View {
    var body: some View {
        Button(
            action: { test = nil },
            label: {Text("test = nil")}
        )
    }
}

As far as I can see weak references are only allowed in closures so that wasn't the answer. Putting the reference into another object meant that both could be released.

I added a ContentView because it makes it easier to play with and I added a print to the pipeline to see what was happening. The computed name is probably not necessary, it just made it look the same as you had. I also removed the Set, it's probably useful but I haven't worked out when.

Michael Salmon
  • 1,056
  • 7
  • 15
  • Thanks. It seems impractical at first to not be able to assign to variables in `self` but I guess I have to get used to it or use a sink. The set is great when you have many different subscribers that need to be stored. Having a dozen `AnyCancellable` variables seems impractical – SwiftedMind Sep 18 '19 at 17:07
  • Maybe it's time for you to write a bug report or at least leave a negative feedback. – Michael Salmon Sep 19 '19 at 06:51
  • I probably should because this is even more inconvenient when you try to observe changes to those properties. Having them in a different class requires you to add all kinds of boilerplate code to observe changes to it. But I simply don't want to believe that Apple hasn't thought of this. There has to be a way to do it – SwiftedMind Sep 19 '19 at 12:34
  • @MichaelSalmon You can use `.store` when `Set` is optional see https://stackoverflow.com/a/60027659/4067700 – Victor Kushnerov Feb 03 '20 at 08:29
  • Doesn't this implicitly capture self via `self.nameStore`? – Marty Aug 02 '23 at 23:26
0

You should remove stored AnyCancellable from disposeBag to release Test instance.

import UIKit
import Combine

private var disposeBag: Set<AnyCancellable> = Set()

class Test {
    public var name: String = ""


    deinit {
        print("deinit")
    }

    init(publisher: CurrentValueSubject<String, Never>) {
        publisher.assign(to: \.name, on: self).store(in: &disposeBag)
    }
}

let publisher = CurrentValueSubject<String, Never>("Test")

var test: Test? = Test(publisher: publisher)
disposeBag.removeAll()
test = nil

or use optional disposeBag

import UIKit
import Combine

class Test {
    public var name: String = ""
    private var disposeBag: Set<AnyCancellable>? = Set()

    deinit {
        print("deinit")
    }

    init(publisher: CurrentValueSubject<String, Never>) {
        guard var disposeBag = disposeBag else { return }
        publisher.assign(to: \.name, on: self).store(in: &disposeBag)
    }
}

let publisher = CurrentValueSubject<String, Never>("Test")

var test: Test? = Test(publisher: publisher)
test = nil
Victor Kushnerov
  • 3,706
  • 27
  • 56
  • I'm missing something here. I don't get why setting `disposeBag` as optional fixes the reference cycle. Can you help me understanding this? Thanks. – superpuccio Dec 02 '20 at 21:24
0

In addition to previously recommended way of using @Published property or .sink() there is another way to break strong reference cycle.

From documentation:

The Subscribers/Assign instance created by this operator maintains a strong reference to object, and sets it to nil when the upstream publisher completes (either normally or with an error).

So sending completion event when you don't need updates from publisher will break reference cycle.

In this case:

publisher.send(completion: .finished)
Stacy Smith
  • 490
  • 5
  • 11
0
import protocol Combine.Publisher
import class Combine.AnyCancellable

fileprivate struct RootUnownedWrapper<R: AnyObject> {
 unowned var root: R
 init(_ root: R) { self.root = root }
}

fileprivate struct RootWeakWrapper<R: AnyObject> {
 weak var root: R!
 init(_ root: R) { self.root = root }
}

extension Publisher where Self.Failure == Never {
 typealias TRootKeyPath<T> = ReferenceWritableKeyPath<T, Self.Output>
 fileprivate typealias TWeakKeyPath<T: AnyObject> = WritableKeyPath<RootWeakWrapper<T>, T>
 func assignWeakly<Root: AnyObject>(to keyPath: TRootKeyPath<Root>, on object: Root) -> AnyCancellable {
  
  let wrapped = RootWeakWrapper(object)
  let wkp: TWeakKeyPath<Root> = \.root
  return assign(to: wkp.appending(path: keyPath), on: wrapped)
 }
 
 fileprivate typealias TUnownedKeyPath<T: AnyObject> = WritableKeyPath<RootUnownedWrapper<T>, T>
 
 func assignUnowned<Root: AnyObject>(to keyPath: TRootKeyPath<Root>,on object: Root) -> AnyCancellable {
  let wrapped = RootUnownedWrapper(object)
  let wkp: TUnownedKeyPath<Root> = \.root
  return assign(to: wkp.appending(path: keyPath), on: wrapped)
 }
}
  • In Combine framework there are Publishers that cannot be sent completion message to relinquish target object instance like .send(completion: .finish) or error message so implementation of. assign(_) based on weak or unowned target objects is sometimes and quite often becomes very handy and useful. – Anton Kalinin Sep 14 '22 at 16:13