Closed Bug 1119564 Opened 5 years ago Closed 5 years ago

TabBarViewController leaks with NotificationCenter reference

Categories

(Firefox for iOS :: General, defect)

All
iOS 7
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: bnicholson, Assigned: bnicholson)

Details

Attachments

(1 file)

46 bytes, text/x-github-pull-request
wesj
: review+
Details | Review
Any blocks used as registered NotificationCenter callbacks are strongly held by the NotificationCenter. If we use a strong self inside of that block, that means we'll never be deallocated, and TabBarViewController is never deinited for this reason. We should make sure to use an unowned self to prevent the leak.
Attached file Pull request
Attachment #8546247 - Flags: review?(wjohnston)
Status: NEW → ASSIGNED
Comment on attachment 8546247 [details] [review]
Pull request

I'm still surprised viewWillDisappear isn't called here, but the change seems find regardless..
Attachment #8546247 - Flags: review?(wjohnston) → review+
You were right to be surprised since viewWillDisappear *is* called. The problem is that the notificationToken apparently holds a strong ref to the observer, so even though we've removed the observer from the notification center, we still have a controller -> token -> observer -> block -> controller cycle. 

Setting the notificationToken to nil in viewWillDisappear is another way to fix this, but I think we should just get in the habit of always passing unowned self to the notification center...or not using the notification center at all with blocks. This nice post explains why: http://sealedabstract.com/code/nsnotificationcenter-with-blocks-considered-harmful/
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.