888053 - Under certain conditions the top sites thumbnails leak database cursors

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Chris Kitching [:ckitching]]

Starting with an Awesomescreen with one or more gaps in the top sites display (That is, you have not yet visited enough sites for Fennec to have images to display in all the slots), tap on such a slot to add a site there. If you select a site for which a thumbnail is not available, the database Cursor goes out of scope without having close() called on it, creating a memory leak.
The culprit seems to be an if statement in TopSitesView, around line 650, which causes the function to return early without closing the Cursor.

Comment 1

[Chris Kitching [:ckitching]]

Attachment: Fix for the Cursor leak described

The fix!

Comment 2

[Brian Nicholson (:bnicholson)]

Comment on attachment 768615 [details] [diff] [review]
Fix for the Cursor leak described

Review of attachment 768615 [details] [diff] [review]:
-----------------------------------------------------------------

Nice find!

::: mobile/android/base/widget/TopSitesView.java
@@ +662,5 @@
> +                                    bitmap = BitmapUtils.decodeByteArray(b);
> +                                }
> +                            }
> +                        } finally {
> +                            if(c != null) {

Nit: space before "("

Comment 3

[Chris Kitching [:ckitching]]

Attachment: Bug 888053 - Fix cursor leak in TopSitesView r=bnicholson

Nit-picked.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/982d07833360

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/982d07833360

Comment 6

[Scoobidiver (away)]

Bug 760384 which depends on this bug is tracked for 23.0.