-1

Edit (after discussion):

It's a common practice in SwiftUI to have a View (struct) hold an ObservableObject. Since a struct is not a reference type, it's easy to forget that is holds one.

The questions should be really:

  1. Can an indirect references cause a retain cycle?
  2. Can a retain cycle involve a single pointer

The answer to both is yes.

  1. struct -> class -> closure{struct.class}
class ViewModel: ObservableObject
...
struct MyView: View {
    @ObservedObject var viewModel: ViewModel
    var body: some View {
        Button("Action") {
            viewModel?.handler {
                // ViewModel is retained here through self:
                // (MyView.viewModel)
                self.someAction()
            }
        }
    }
}

In SwiftUI, it's not a good practice to 'pass yourself' to your ViewModel

  1. Class holding an instance var pointing to itself
    (discussed here How does ARC deal with circular linked lists when the external head reference is removed?)
class A {
    var next: A
}
let a = A()
a.next = a // Retain cycle of an object to itself.

(Original question)

Why is holding a reference to a closure cause a memory leak even if the closure does not retain anything?

(This started with a SwiftUI issue, but I'm not sure it's related really)

So here's a simple Manager (ViewModel) that holds a closure.

class Manager: ObservableObject {
    
    private var handler: (() -> Void)?
    
    deinit {
        print("Manager deallocated")
    }
    
    func shouldDismiss(completion: @escaping () -> Void) {
        handler = completion
        DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
            self.handler?()
        }
    }
}

Here's a view, using it:

struct LeakingView: View {
    
    @ObservedObject var manager: Manager
    
    let shouldDismiss: () -> Void
    
    var body: some View {
        Button("Dismiss") { [weak manager] in
            manager?.shouldDismiss {
                self.shouldDismiss()
            }
        }
    }
}

Here's a ViewController activating the flow:

class ViewController: UIViewController {

    var popup: UIViewController?
    
    @IBAction func openViewAction(_ sender: UIButton) {
        
        let manager = Manager()
        
        let suiView = LeakingView(manager: manager) { [weak self] in
            self?.popup?.view.removeFromSuperview()
            self?.popup?.dismiss(animated: true)
            self?.popup = nil
        }
        
        popup = LKHostingView(rootView: suiView)
        
        
        popup?.view.frame.size = CGSize(width: 300, height: 300)
        view.addSubview(popup!.view)
    }
    
}

When running this, as long as the manager is holding the closure, it will leak and the Manager will not get released.
Where is the retain cycle?

bauerMusic
  • 5,470
  • 5
  • 38
  • 53

2 Answers2

5

Why is holding a reference to a closure cause a memory leak even if the closure does not retain anything?

Because the closure does retain something. It retains self.

For example, when you say

        manager?.shouldDismiss {
            shouldDismiss()
        }

The second shouldDismiss means self.shouldDismiss and retains self. Thus the LeakingView retains the Manager but the Manager, by way of the closure, is now retaining the LeakingView. Retain cycle!

This is why people say [weak self] in at the start of such closures.

matt
  • 515,959
  • 87
  • 875
  • 1,141
  • 2
    In this case, if `shouldDismiss` is immutable, you can just capture that directly, instead of all of `self`. – Alexander Jul 25 '22 at 17:25
  • Yes, that's why I said "why people say `[weak self]`". I did _not_ say that this is what _you_ should say. I was just saying that it gets used a lot as a sort of catch-all way of solving the issue. It's way over-used. – matt Jul 25 '22 at 17:38
  • 2
    Ok, but in this case, `self` is a `struct`, can we really 'retain' it, is it not just copied? And if so, what can we do (since we cannot use `weak` for a non reference var)? – bauerMusic Jul 25 '22 at 18:15
  • @matt *I* know that, but you should explain it in your answer for OP – Alexander Jul 25 '22 at 18:45
  • 1
    The question was where the retain cycle is. The question wasn't what to do about it. – matt Jul 25 '22 at 18:48
  • 1
    Ok, so an retain cycle between an object and a struct is possible: https://stackoverflow.com/questions/42632122/retain-cycle-between-class-and-struct That was really the main issue here – bauerMusic Jul 25 '22 at 18:51
  • @bauerMusic Certainly. Why not? – matt Jul 25 '22 at 19:00
  • 1
    _"Because the closure does retain something. It retains self."_ - Is it really 'self' that is being retained, or to pointer it holds? In that case, the `Manager` is retaining a pointer to itself by a `Struct`. There is only _one_ pointer here no? – bauerMusic Jul 26 '22 at 04:56
  • I guess this is what is happening (simplified): `class A { var a: A? }`; `let a = A()`; `a.a = a`. An object retaining itself. It will not get released until `a.a = nil`. – bauerMusic Jul 26 '22 at 05:09
2

I would suggest that you might not want to use [weak self] pattern here. You are defining something that should happen ½ second after the view is dismissed. So you probably want to keep that strong reference until the desired action is complete.

One approach is to make sure that Manager simply removes its strong reference after the handler is called:

class Manager: ObservableObject {
    private var handler: (() -> Void)?
    
    func shouldDismiss(completion: @escaping () -> Void) {
        handler = completion
        DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
            self.handler?()
            self.handler = nil
        }
    }
}

Alternatively, if the use of this completion handler is really limited to this method, you can simplify this to:

class Manager: ObservableObject {
    func shouldDismiss(completion: @escaping () -> Void) {
        DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
            completion()
        }
    }
}

Or

func shouldDismiss(completion: @escaping () -> Void) {
    DispatchQueue.main.asyncAfter(deadline: .now() + 0.5, execute: completion)
}

All of these approaches will make sure that Manager does not keep any strong references beyond the necessary time.

Now, the caller can choose to use [weak self] if it wants (i.e., if it doesn’t want the captured references to even survive ½ second), but if you really want the “do such-and-such after it is dismissed”, it probably wouldn’t use weak references there either. But either way, it should be up to the caller, not the manager. The manager should just ensure that it doesn’t hang on to the closure beyond the point that it is no longer needed.

Rob
  • 415,655
  • 72
  • 787
  • 1,044
  • The takeaway is really that holding a closure in this settings is not a good practice. The code is just a sample, the ½ seconds just symbolizes some async task. In the original code (which I cannot share and did not originally wrote) there's a pointer to a closure that is called in too many places. The original reference cycle was caused by a `@State var` called inside a Manager's completion inside a `View` (like the sample provided). Looks perfectly innocent until things got leaky. – bauerMusic Jul 26 '22 at 03:13
  • Yeah, I figured you may have simplified it for the sake of the question, which is why I didn't jump directly to the streamlined examples. – Rob Jul 26 '22 at 04:16
  • 1
    Just to be clear, a `ViewModel` cannot actually `retain` a `Struct` anymore than it can retain an `Int`, what it retains is its own (`ViewModel`'s) reference? – bauerMusic Jul 26 '22 at 04:51
  • 1
    Correct, a struct cannot be retained. – Rob Jul 26 '22 at 07:15