1

When I perform remove(at: [index]) on a UIImage array, within a collectionView-didSelectItemAt/UIAlertController, I end up having a memory leak. (Note: The array is actually apart of a class I defined.)

The issue was very hard to find because it appears to work at the time of deletion. To be more specific, after I delete an image from the array and print the contents, it's gone and memory looks fine. But later, when I'm done with that object, the objects deinit method never gets called.

Note: I've also tried executing the code on the main queue.

class ExampleObj {
    var vehicleImages: [UIImage]

    init() {
        self.vehicleImages = []
    }

    deinit() {
        print("Object deinitialized")
    }
}

func collectionView(_ collectionView: UICollectionView, didSelectItemAt indexPath: IndexPath) {
    let alert = UIAlertController(title: "Delete Image", message: "", preferredStyle: .alert)

    alert.addAction(UIAlertAction(title: "Delete Image", style: .destructive, handler: { (action) in
        alert.dismiss(animated: true, completion: nil)
        // Delete image
        self.exampleObj.vehicleImages.remove(at: indexPath.row)
    }))

    alert.addAction(UIAlertAction(title: "Return", style: .default, handler: { (action) in
        alert.dismiss(animated: true, completion: nil)
    }))
    self.present(alert, animated: true, completion: nil)
}
MarvelS
  • 11
  • 3
  • 1
    If you use `handler: { [weak self] (action) in //code }` does that solve your memory leak issue? – Dan O'Leary Aug 25 '19 at 22:40
  • I don't believe it would; I removed all the code within the action and tried running and everything worked fine. @DanO'Leary – MarvelS Aug 26 '19 at 12:07
  • I didn't think it was necessary to add the deinit, my mistake. And I'm not sure how else to explain the nature of the leak... when I'm done with that object, the objects deinit method never gets called and therefore the object doesn't get deallocated - leak. What's weird is if I move (self.exampleObj.vehicleImages.remove(at: indexPath.row)) outside the alert controller block (and keep everything else the same) I don't see the leak @Rob – MarvelS Sep 06 '19 at 13:02
  • 1
    In your example code, `ExampleObj.deinit` shouldn't be called. You're not removing `ExampleObj`. You're removing an image that is contained inside of `ExampleObj`. What behavior do you believe should happen here, and what behavior does happen (and how are you determining that this behavior happens)? – Rob Napier Sep 06 '19 at 13:08
  • @RobNapier Please read the post a little more carefully: "after I delete an image from the array and print the contents, it's gone and memory looks fine. ~But later, when I'm done with that object,~ the objects deinit method never gets called." So what I mean when I say "later, when I'm done with that object" I mean I've done everything I needed in the VC that the object was created in and I go back (dismissing said VC) the object (and VC) should be deallocated, but they're not. – MarvelS Sep 06 '19 at 13:34
  • That's also why I said it was hard to find; because it ("it" being the leak) doesn't happen at the time of deletion. @RobNapier – MarvelS Sep 06 '19 at 13:38
  • @Rob did you just read swift memory leaks 101 to try to answer this question? Your advice is literally the definition of a memory leak (RE: "might have other references", "Maybe you have some strong reference cycle keeping that around") and to use the debug memory graph. – MarvelS Sep 06 '19 at 15:16
  • @Rob And how are you going to say it has nothing to do with the array given that when I move/remove (self.exampleObj.vehicleImages.remove(at: indexPath.row)) (and that ALONE) the problem disappears – MarvelS Sep 06 '19 at 15:16
  • First, "There are two completely unrelated and separate issues" is kind of redundant. Second, What I was trying to say was that the stuff you're pointing out is in every single page result when you google "swift memory leaks" and is not helpful and just adds clutter to the post. I also forgot to add to the list "Maybe you have some strong reference cycle keeping that around" @Rob – MarvelS Sep 06 '19 at 15:56
  • @DanO’Leary - You are absolutely right that this would fix the problem with `ExampleObj` (and the view controller) from not getting deallocated, but it wouldn’t address the root cause of the problem, the strong reference cycle between the alert controller and the alert actions. – Rob Sep 06 '19 at 19:10

1 Answers1

0

The problem is the presence of the reference to alert within your UIAlertAction closure:

alert.addAction(UIAlertAction(title: "Delete Image", style: .destructive, handler: { (action) in
    alert.dismiss(animated: true, completion: nil)
}))

This is not only not needed (alerts are dismissed automatically, so calling dismiss yourself is redundant and potentially problematic), but this reference to alert within this closure is also introducing a strong reference cycle between the alert controller, the alert action, the closure, and then back to the alert controller.

Worse, when you add a reference to self in there:

alert.addAction(UIAlertAction(title: "Delete Image", style: .destructive, handler: { (action) in
    alert.dismiss(animated: true, completion: nil)
    self.exampleObj.vehicleImages.remove(at: indexPath.row)
}))

Now this strong reference cycle between the alert controller and its alert action is now introducing a strong reference to self, your view controller, which will prevent its properties from being deallocated either.

So just remove these alert references from within the closures, and the problem will go away:

alert.addAction(UIAlertAction(title: "Delete Image", style: .destructive) { _ in
    self.exampleObj.vehicleImages.remove(at: indexPath.row)
})

alert.addAction(UIAlertAction(title: "Return", style: .default))

If you want, you could also make self a weak reference:

alert.addAction(UIAlertAction(title: "Delete Image", style: .destructive) { [weak self] _ in
    self?.exampleObj.vehicleImages.remove(at: indexPath.row)
})

That way, even if you accidentally introduced a strong reference cycle between the alert controller and its actions, at least self wouldn’t get caught up in that strong reference cycle. (But it’s worth noting that [weak self] is not enough to solve the strong reference cycle, but just keeps self from being involved. You really want to remove those alert references to avoid the strong reference cycle in the first place.)


FWIW, you can use the “Debug Memory Graph” feature to find what is keeping a strong reference to your object, trace that back, and ultimately if you use the “Malloc stack” feature, you can see precisely where this root strong reference was established. In this case, I presented an alert controller (with that strong reference to both alert and self in the closure), then dismissed the view controller in question, tapped on the “Debug Memory Graph” button, and it shows us that ExampleObj, the view controller, the alert controller, and the alert action are all still in memory:

enter image description here

It’s admittedly tough to follow that graph, but I focus on objects that I created, and I can trace it back from the ExampleObj to the view controller to the alert action, and even to the line of code where alert created the strong reference to the view controller. I’ve added those arrows to show how I tracked it back. All of this admittedly doesn’t tell you how to fix the problem, but it lets you track it back, at which point you have a head start in diagnosing the issue.

Rob
  • 415,655
  • 72
  • 787
  • 1,044