1

I was surprised to recently learn self references within a self reference (say a method) can still cause reference cycles (or prevent deinit). This may also be a Combine issue.

Say we have a class like this:

final class LogicController {
    struct State { let summary: String }
    
    @Published var state: State?
    
    private let local = "local"
    private var tokens: Set<AnyCancellable> = []
    
    init() {
        let firstPublisher = $firstPublisher
        let secondPublisher = $secondPublisher
        
        Publishers.CombineLatest(firstPublisher, secondPublisher)
            .map(makeState)
            .assign(to: \.state, on: self)
            .store(in: &tokens)
    }
    
    func makeState(first: String, second: String) -> State {
        State(summary: "\(first)+\(second)+\(local)")
    }
}

(Ignore the fact that assign leaks, it's fixed with an overload.)

I use this pattern to reduce global state into local state for view controllers. In the memory debugger, this can show a cycle or instances left behind due to the capture of makeState in map(makeState). This is a pretty obvious "observer" capture, so we're used to doing this for notifications. So we can make the capture explicit: map { [unowned self] in self.makeState($0) }. Not as elegant, but should be safe, right? Apparently not. The additional capture of self within makeState itself also creates a cycle. Obviously, with such a simple makeState we can move its logic directly into the map so the unowned capture is effective and the instance can properly deinit. For more complex logic, a local closure can be used with an explicit self capture that is also safe, if rather noisy and non obvious.

So, my questions are:

  1. Is my analysis correct?
  2. Is there a more elegant way to deal with these internal captures?
  3. Is there a better way to use Combine to do this without the local token storage that causes the cycle? There's a new assign operator that can assign to a @Published property directly, but I'd need to backport it to iOS 13 and it isn't clear how it works.
2

I'm not sure what you mean by "The additional capture of self within makeState itself also creates a cycle." I'd expect your rewrite using unowned to work fine; the implicit use of self within makeState does not do any sort of capturing.

3

That's what I thought, but it's not borne out by my comparisons using Xcode's memory debugger. There's no visible reference cycle, but instances are kept alive even after rebuilding the tree that creates them. The only way I found to ensure I ended up with a single active reference was to capture the logic of the method as unowned. I could either turn it into a local closure that captured [unowned self] or I could modify the instance method to take an explicit instance, as below.

func makeState(using instance: LogicController, first: String, second: String) -> State {
    State(summary: "\(first)+\(second)+\(instance.local)")
}

This works when I explicitly capture self for each map:

Publishers.CombineLatest(firstPublisher, secondPublisher)
    .map { [unowned self] in (self, $0.0, $0.1) }
    .map { [unowned self] in self.makeState(using: $0.0, first: $0.1, second: $0.2) }
    .assign(to: \.state, on: self)
    .store(in: &tokens)

However, attempting a single capture doesn't:

call { [unowned self] in
    Publishers.CombineLatest(firstPublisher, secondPublisher)
        .map { (self, $0.0, $0.1) }
        .map (self.makeState)
        .assign(to: \.state, on: self)
        .store(in: &tokens)
}

I really not sure why there's a difference between the two.

Actually, testing again, a direct unowned capture works fine to call the unmodified instance method, but using my wrapping closure does not. I really don't understand that part.

I think this comes down to the retain cycle created by Combine when storing the cancellation token in the same instance that's being captured. The new assign(to:) would fix that part of it, but I don't know how to replicate it on iOS 13.

In the end I tried to write some unit tests to replicate what I'm seeing in the app, but they don't.

Unit tests ```swift final class CaptureTests: BaseTestCase { final class Hunter { struct State { let summary: String } 
    @Published var state: State?
    
    private let local = "local"
    private var tokens: Set<AnyCancellable> = []
    
    init(firstNaive first: String) {
        let first = Just(first)
        let second = Just("second")
        
        Publishers.CombineLatest(first, second)
            .map(makeState)
            .assign(to: \.state, on: self)
            .store(in: &tokens)
    }
    
    init(firstDirect first: String) {
        let first = Just(first)
        let second = Just("second")
        
        Publishers.CombineLatest(first, second)
            .map { [unowned self] in self.makeState(first: $0.0, second: $0.1) }
            .assign(to: \.state, on: self)
            .store(in: &tokens)
    }
    
    init(firstWrapped first: String) {
        let first = Just(first)
        let second = Just("second")
        
        toCaptureWeakly { [unowned self] in
            Publishers.CombineLatest(first, second)
                .map(self.makeState)
                .assign(to: \.state, on: self)
                .store(in: &tokens)
        }
    }
    
    @available(iOS 14.0, *)
    init(firstAssign first: String) {
        let first = Just(first)
        let second = Just("second")
        
        Publishers.CombineLatest(first, second)
            .map { [unowned self] in self.makeState(first: $0.0, second: $0.1) }
            .assign(to: &$state)
    }
    
    func makeState(first: String, second: String) -> State {
        State(summary: "\(first)+\(local)+\(second)")
    }
}
    
func testNaiveCapture() {
    // Given
    var hunter: Hunter? = Hunter(firstNaive: "first")
    weak var ref = hunter
    var state: Hunter.State?
    let expect = expectation(description: "received value")
    
    store {
        hunter!.$state.sink { state = $0; expect.fulfill() }
    }
    
    waitForExpectations(timeout: timeout)
    
    // When
    hunter = nil
    
    // Then
    XCTAssertNil(ref)
    XCTAssertNotNil(state)
}

func testDirectCapture() {
    // Given
    var hunter: Hunter? = Hunter(firstDirect: "first")
    weak var ref = hunter
    var state: Hunter.State?
    let expect = expectation(description: "received value")
    
    store {
        hunter!.$state.sink { state = $0; expect.fulfill() }
    }
    
    waitForExpectations(timeout: timeout)
    
    // When
    hunter = nil
    
    // Then
    XCTAssertNil(ref)
    XCTAssertNotNil(state)
}

func testWrappedCapture() {
    // Given
    var hunter: Hunter? = Hunter(firstWrapped: "first")
    weak var ref = hunter
    var state: Hunter.State?
    let expect = expectation(description: "received value")
    
    store {
        hunter!.$state.sink { state = $0; expect.fulfill() }
    }
    
    waitForExpectations(timeout: timeout)
    
    // When
    hunter = nil
    
    // Then
    XCTAssertNil(ref)
    XCTAssertNotNil(state)
}

func testNoCapture() {
    // Given
    var hunter: Hunter? = Hunter(firstWrapped: "first")
    weak var ref = hunter
    var state: Hunter.State?
    let expect = expectation(description: "received value")
    
    store {
        hunter!.$state.sink { state = $0; expect.fulfill() }
    }
    
    waitForExpectations(timeout: timeout)
    
    // When
    // hunter = nil
    
    // Then
    XCTAssertNotNil(ref)
    XCTAssertNotNil(state)
}

@available(iOS 14.0, *)
func testNewCapture() {
    // Given
    var hunter: Hunter? = Hunter(firstAssign: "first")
    weak var ref = hunter
    var state: Hunter.State?
    let expect = expectation(description: "received value")
    
    store {
        hunter!.$state.sink { state = $0; expect.fulfill() }
    }
    
    waitForExpectations(timeout: timeout)
    
    // When
    hunter = nil
    
    // Then
    XCTAssertNil(ref)
    XCTAssertNotNil(state)
}

}

</details>
4

self.makeState is essentially short for { [self] in self.makeState($0, $1..., $N) }, so you're not avoiding the capture in your short version even though the method itself doesn't use it. You can see the difference by making your method static, in which case there's no instance involved.

5

Wait, so the [self] in your expanded version isn't the self I capture in the call closure?

6

I’m pretty sure that, even if it’s the same, it needs to be recaptured, and self.makeState will strongly recapture self.

What’s interesting to me is that .assign(to: \.state, self) doesn’t induce cycle, since it explicitly states that it’ll hold a strong reference to self. Maybe the publisher finishes in your example, and causes assign to release it?

1 Like
7

I have an overload of assign that captures weakly.

1 Like
8

That's what I was wondering at the beginning, when I mentioned a capture. @jrose, is this the case?

9

The way capture lists work is that { [self] in self.makeState($0, $1..., $N) } is expanded in the AST to, basically, { [let self = self] in self.makeState($0, $1..., $N) }. So when you drop something in the capture list, you're creating a new binding which is initialized from the 'old' name. In the case of reference types, the reference is implicitly strong if you leave off the specifier. So the direct answer to your question is no, the self capture in Jordan's expanded version creates a new, strong reference to self which shadows the unowned reference.

I believe it's the case (someone correct me if I'm wrong), that during the course of execution of any method of a reference type, the body has access to a strong reference to self. Thus, when a bound method is formed (e.g., self.makeState), we must bind a strong reference to self along with the method reference so that invoking the bound method can provide a strong reference to the body.

In the case where you immediately apply makeState, we still form that strong reference. However, in this situation the strong reference only lives as long as the invocation lasts, and is released as soon as the method finishes executing.

10

But since my usage is part of a Combine stream kept alive by the same instance, the invocation has the same lifetime if it's a strong capture. Right?

11

The duration of the invocation of makeState is (mostly) unrelated to the lifetime of the Combine stream (except that the Combine stream 'kicks off' the invocation of makeState, so the stream must last at least long enough to do so). E.g., if we wrote:

Publishers.CombineLatest(firstPublisher, secondPublisher)
    .map { [unowned self] in (self, $0.0, $0.1) }
    .map {
        let controller = LogicController()
        controller.makeState(using: $0.0, first: $0.1, second: $0.2) 
    }
    .assign(to: \.state, on: self)
    .store(in: &tokens)

then it should be apparent that controller does not live on past each invocation of the closure passed to the second map (i.e., each time firstPublisher or secondPublisher produce a value).

When either publisher produces a value, Combine will (eventually) invoke the closure provided to map. We then allocate a LogicController and store a strong reference in controller. Then, we invoke controller.makeState, which forms an additional strong reference to the same LogicController, which is passed to the method as self. Then, makeState returns, and self is released. Then, the map closure returns, and controller is released. Finally, LogicController can be deallocated.

Looking back at the version using unowned self, the analysis is similar, except that there's no 'outer' strong reference:

Publishers.CombineLatest(firstPublisher, secondPublisher)
    .map { [unowned self] in (self, $0.0, $0.1) }
    .map { [unowned self] in self.makeState(using: $0.0, first: $0.1, second: $0.2) }
    .assign(to: \.state, on: self)
    .store(in: &tokens)

When either publisher produces a value, Combine eventually executes the closure passed to map. To execute self.makeState, we check whether self has been deallocated (and crash if so). Otherwise, we form a new strong reference to self and pass this reference to makeState.

During the invocation of makeState, self cannot be deallocated since there is an extant strong reference to self. Eventually, makeState returns, and self is released. At this point, there may or may not be strong references to self outstanding, but the Combine stream is not holding onto any.

12

I'm sorry, but this explanation doesn't really make sense to me. Combine isn't storing an invocation (at least as I'm familiar with the term from Obj-C), but is instead storing a closure as part of the operator. That is, map takes a closure, which it then keeps a reference to to call whenever it receives a value. That capture is strong and therefore care must be taken to ensure proper self capture.

As far as I understand it, capturing self.makeState using an unowned reference doesn't capture the content of that method with the same self reference, resulting in a strong capture of self as part of the stream which is kept alive by the same instance, causing the reference cycle.

13

Ah, I was using "invocation" more generally to refer the the period of time during which makeState is actively executing—sorry that was unclear. The main point I was trying to make is that in both cases (the one where you form a bound method reference to self.makeState, and the one where you immediately apply self.makeState), you are forming a strong reference to self from your unowned reference to self. The difference is that in the case where makeState is immediately applied, the strong reference only lasts for the duration of the call to makeState.

Yeah, this is right. Forming a bound method reference creates a new (strong) reference to the self param. In the past, there's been discussion around extending the "explicit self required" rules to alert the user when they pass a bound method reference to an @escaping parameter using implicit self, for precisely the same reason.

2 Likes