0

I'm creating a custom table view cell, which allows the user to add, take, or view uploaded photos.

I discovered that this cell stays in memory forever even after the table view's dismissal, creating weird memory graph. I want the cell to be dismissed properly, but I have a hard time understanding what is going on.

The graph shows that my cell is being strongly referenced by a addPhotoTapAction.context.

addPhotoTapAction: ((ItemInfoCell) -> Void)? is the cell's class variable, used to store the closure handling user input. The closure is defined in the view controller:

let infocell = tableView.dequeueReusableCell(withIdentifier: K.infocellID) as! ItemInfoCell
    if item?.imageUrl == nil {
        self.imageManager.actionController?.actions[2].isEnabled = false
    } else {
        self.imageManager.actionController?.actions[2].isEnabled = true
    }
    infocell.addPhotoTapAction = { [unowned self] _ in
        infocell.addPhotoButton.isEnabled = false
        self.imageManager.pickImage(self) { [weak self] image in
            self?.imageToSave = image
            infocell.itemPhoto.image = self?.imageToSave
            infocell.addPhotoButton.tintColor = UIColor(ciColor: .clear)
            infocell.addPhotoButton.isEnabled = true
            self?.imageManager.actionController?.actions[2].isEnabled = true
        }

The pickImage method is shown below. It's used to present action controller with image picker options (take photo or choose from lib):

func pickImage(_ viewController: UIViewController, _ callback: @escaping ((UIImage) -> ())) {
    picker.delegate = self
    picker.mediaTypes = ["public.image"]
    picker.allowsEditing = true
    pickImageCallback = callback
    self.viewController = viewController
    actionController!.popoverPresentationController?.sourceView = viewController.view
    viewController.present(actionController!, animated: true, completion: nil)
}

...and the callback is stored to be used in picker's didFinishPickingMediaWithInfo call:

func imagePickerController(_ picker: UIImagePickerController, didFinishPickingMediaWithInfo info: [UIImagePickerController.InfoKey : Any]) {
    picker.dismiss(animated: true, completion: nil)
    if let image = info[.editedImage] as? UIImage {
        let squareImage = makeSquare(image)
        pickImageCallback?(squareImage)
    } else if let image = info[.originalImage] as? UIImage {
        let squareImage = makeSquare(image)
        pickImageCallback?(squareImage)
    }
    viewController = nil
}

I've tried to manually set variable with closure to nil, then switched from [weak self] to [unowned self] to the combination of both. No luck.

I think that either the pickImage(self), or using the class' properties within the closure are creating a strong reference even when using [weak/unowned] capture lists, but I'm still not sure and not being able to fix it.

Update: ItemInfoCell class' code

class ItemInfoCell: UITableViewCell {

@IBOutlet weak var itemPhoto: UIImageView!
@IBOutlet weak var itemLabel: UILabel!
@IBOutlet weak var addPhotoButton: UIButton!

var addPhotoTapAction: ((ItemInfoCell) -> Void)?

override func awakeFromNib() {
    super.awakeFromNib()
}

override func setSelected(_ selected: Bool, animated: Bool) {
    super.setSelected(selected, animated: animated)
}

@IBAction func takePhoto(_ sender: Any) {
    if let addPhoto = self.addPhotoTapAction {
        addPhoto(self)
    }
}

}

  • what is imageManager ? – Jawad Ali May 04 '20 at 20:15
  • 1
    It looks like your imageManager has a strong reference to the viewController, and the viewController has a strong reference to the imageManager. – Rob C May 04 '20 at 20:16
  • ImageManager is the separate class hosting the methods below @Rob - thought about it, but the imageManager is being dismissed - so the cell is only entity that stays in memory – Cyril Semenov May 04 '20 at 20:42
  • Can you post your 'ItemInfoCell' class? – elliott-io May 04 '20 at 20:57
  • @elliott-io, sure! Added it above – Cyril Semenov May 04 '20 at 20:59
  • update this line `self.imageManager.pickImage(self) { [weak self] image in` with `self.imageManager.pickImage(self) { [weak self] [weak infocell ] image in` – Jawad Ali May 04 '20 at 21:41
  • Would you try to mark infocell as weak too? Add it to capture list when you define “infocell.addPhotoTapAction = { [unowned self weak infocell] _in ...}” You access that cell inside it’s callback. That could be the reason. – Ondřej Korol May 04 '20 at 22:08
  • @OndřejKorol, that was the reason, thank you! I've also tried to pass the cell reference at the _ mark in "[unowned self] _in {". I thought that Swift will properly deallocate it, but it didn't. Passing the cell as [unowned self, weak infocell] fixes the issue – Cyril Semenov May 05 '20 at 05:15
  • @CyrilSemenov Glad to hear that:) In this case, could you please accept the answer below, so other people can see the solution too? – Ondřej Korol May 05 '20 at 10:55

2 Answers2

1

The problem is that you access the infocell inside it's callback. If you use variable inside own callback, you should mark it as a weak by adding it to the capture list.

In your code it should be like this:

   infocell.addPhotoTapAction = { [unowned self, weak infocell] _ in
      ...
    }
Ondřej Korol
  • 594
  • 6
  • 19
0

In your code, there is a strong reference from infocell to closure and inside closure you are referring to infocell i.e. both have a strong reference to each other.

infocell.addPhotoTapAction = { [unowned self] _ in
    infocell.addPhotoButton.isEnabled = false
     ...
}

This leads to a retain cycle and doesn't allow the cell to be deallocated.

Since you are already using capture list for self, you can easily solve this issue by adding infocell to the capture list, like so:

infocell.addPhotoTapAction = { [weak self, weak infocell] _ in
    guard let self = self else { return }
    infocell.addPhotoButton.isEnabled = false
    ...
}

Generally speaking, it is suggested to use unowned only when you are completely sure that the reference will not become nil before execution of the closure, because using unowned is like forcefully unwrapping an optional value. If it is nil your app will crash. So weak self is generally the safer way to go.

Deeksha Kaul
  • 264
  • 2
  • 7
  • You can refer to [this article](https://medium.com/@deekshakaul1/memory-management-in-swift-6028e6a8976c) to understand how weak and unowned references really work and what could be causing your memory leak – Deeksha Kaul May 28 '20 at 13:15