Swizzling for automatic check

[jazzbox]

Hi, just my 2 cents, because I don‘t want to manually add viewWillDisappear(_:) to all of my view controllers I swizzled the UIViewController method for the check:

import UIKit

private func swizzle(forClass: AnyClass, originalSelector: Selector, swizzledSelector: Selector) {
    let originalMethod = class_getInstanceMethod(forClass, originalSelector)
    let swizzledMethod = class_getInstanceMethod(forClass, swizzledSelector)
    method_exchangeImplementations(originalMethod, swizzledMethod)
}

extension UIViewController {

    class func swizzleForDeallocationCheck() {
        #if DEBUG
            swizzle(forClass: self, originalSelector: #selector(viewWillDisappear(_:)), swizzledSelector: #selector(swizzledViewWillDisappear(animated:)))
        #endif
    }

    @objc
    private func swizzledViewWillDisappear(animated: Bool) {
        #if DEBUG
            self.swizzledViewWillDisappear(animated: animated)

            //print("viewWillDisappear: class \(type(of: self))")

            let delay: TimeInterval = 2.0

            // We don't check `isBeingDismissed` simply on this view controller because it's common
            // to wrap a view controller in another view controller (e.g. a stock UINavigationController)
            // and present the wrapping view controller instead.
            if isMovingFromParentViewController || rootParentViewController.isBeingDismissed {
                let type = type(of: self)
                let disappearanceSource: String = isMovingFromParentViewController ? "removed from its parent" : "dismissed"

                DispatchQueue.main.asyncAfter(deadline: .now() + delay, execute: { [weak self] in
                    assert(self == nil, "\(type) not deallocated after being \(disappearanceSource)")
                })
            }
        #endif
    }

    private var rootParentViewController: UIViewController {
        var root = self
        while let parent = root.parent {
            root = parent
        }
        return root
    }

}

func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplicationLaunchOptionsKey : Any]? = nil) -> Bool {
        UIViewController.swizzleForDeallocationCheck()
        // ...
}

[fastred]

Thanks! I didn't include swizzling because there are some cases in which this check isn't correct, e.g. for view controllers used in UITabBarController – they are kept alive after disappearing from the screen.

[santoshbo]

What happen if VC is hold by other object and on back button click it get pop from the navigation stack? It will consider that VC as a released, Right?

[kgaidis]

@fastred

I would +1 on exploring the ability to swizzle. I might be missing something, but it sounds like swizzling could be lot more maintainable in the long-run.

To fix the special cases, there would be a list (set) of exceptions which skip the deallocation check: UIViewController.swizzleForDeallocationCheck(withExceptions:)

It would be annoying at first to find out all the exception cases, but once done, any new UIViewController would get this behavior without any new dev knowing any better. The assert statement would be improved to notify of a potential error case: "(type) not deallocated after being (disappearanceSource). If you believe this is expected behavior, add this view controller to the exception list blabla....".

As I said, I could be missing some reasons why this is a bad idea, but right now it sounds like it would be a lot more maintainable + "isolated" without littering dch_checkDeallocation() everywhere.

[fastred]

@kgaidis I'm not a big fan of swizzling but I understand some people may prefer this approach. Please feel free to submit a pull request for this change if you want to.

One thing to note is that it would probably be good to either 1) swizzle only view controllers from the current target, or 2) allow passing strings instead of classes to swizzleForDeallocationCheck(withExceptions:) – I expect there may be some leaks in private UIKit view controllers.