Swift - Holding closure reference leaks-CodePudding

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 {
                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?

CodePudding user response：

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.

CodePudding user response：

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()
        }
    }
}

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.