JanGorman / Agrume

🍋 A lemony fresh iOS image viewer written in Swift.
MIT License
790 stars 121 forks source link

Unowned reference in AgrumeCell can cause crash #326

Closed lukeredpath closed 2 years ago

lukeredpath commented 2 years ago

https://github.com/JanGorman/Agrume/blob/dac5b02579de3879d5306b9130b272f99468571d/Agrume/AgrumeCell.swift#L178

This line uses an unowned reference to self, which can cause a crash if the cell is deallocated before the animation completes. Because the completion handler is a global reference that is not retained by the cell, using an unowned reference to self is incorrect here as it’s lifetime can outlive the cell.

Whilst it would be safe to use a strong reference here (as there is no retain cycle), it would be more appropriate to use a weak reference otherwise the animation completion handler would unnecessarily extend the lifetime of the cell when it would otherwise be deallocated.