fulldecent / FDTake

Easily take a photo or video or choose from library
MIT License
319 stars 121 forks source link

FDTakeController deinit is not being called. Seems to be a memory leak for me. #105

Closed Kharauzov closed 4 years ago

Kharauzov commented 7 years ago

Hello Guys, I have such method to show FDTake at my view controller. Code at my ViewController:

fileprivate var fdTakeController: FDTakeController!

 func showFDTakeController() {
        fdTakeController = ImageManager.fdTakeController
        fdTakeController.didFail = { [unowned self] in
            let alert = UIAlertController(title: "Failed", message: "User selected but API failed", preferredStyle: .alert)
            alert.addAction(UIAlertAction(title: "OK", style: .default, handler: nil))
            self.present(alert, animated: true, completion: nil)
        }
        fdTakeController.didGetPhoto = { [unowned self]
            (image: UIImage, info: [AnyHashable : Any]) in
            self.avatarButton.setImage(image, for: .normal)
            self.avatarButton.setImage(image, for: .highlighted)
            self.memberService.imageData = UIImageJPEGRepresentation(image, 0.5)
        }
        self.fdTakeController.presentingView = self.view
        self.fdTakeController.present()
   }

Problem is, that when I go away from my controller, its deinit is being called as it should to be. However I noticed, that deinit of FDTake controller is not called. Could you tell me, maybe I did something wrong at implementation of FDTake at my controller. Because it causes memory leak.

fulldecent commented 7 years ago

You are using [unowned self] which is discussed here http://stackoverflow.com/questions/24320347/shall-we-always-use-unowned-self-inside-closure-in-swift

Your approach seems correct.

A temporary solution may be to use fdTakeController = nil in the deinit. But I don't have any other ideas right now. Hopefully other, more creative people, can find this issue and comment.

Kharauzov commented 7 years ago

@fulldecent Unfortunately, this does not work fdTakeController = nil. I've tried it before already. Okey, maybe someone else will help. But thanks for the quick reply and help in any way.

ivancantarino commented 6 years ago

Have you tried to use [weak self] instead? Then you can handle the self in a week reference cycle. If you want to ensure that self is present without forcing unwrapping then you can instantiate a guard let this = self else { return } then use your this.someFunc() when needed. After dismissing the view, the self retain cycle will be deallocated automatically without holding on any strong reference that could have been captured by the closure. That worked for me, hopefully it'll work for you.

fulldecent commented 5 years ago

This is now in Swift I believe this issue is fixed

fulldecent commented 4 years ago

Closing for now. Major updates in c3450d12a3aaa2325465935c2efad156d76672de

Please reopen if this can be reproduced in the latest version.