The default bug view has changed. See this FAQ.

Audit code that deals with java.nio.Buffer

RESOLVED FIXED in Firefox 15

Status

()

Firefox for Android
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: kats, Assigned: kats)

Tracking

unspecified
Firefox 15
All
Android
Points:
---

Firefox Tracking Flags

(firefox14 wontfix, firefox15 fixed)

Details

Attachments

(1 attachment)

Created attachment 622372 [details] [diff] [review]
Patch

We might be leaking buffers somewhere, see bug 751343 and bug 750965. I audited the relevant code and found some possible double-frees (ScrollbarLayer.finalize(), TextLayer.finalize()) and possible leaks (BufferedCairoImage.setBuffer).
Attachment #622372 - Flags: review?(chrislord.net)

Comment 1

5 years ago
Comment on attachment 622372 [details] [diff] [review]
Patch

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

Looks good to me.

::: mobile/android/base/gfx/TileLayer.java
@@ -198,5 @@
> -
> -            GLES20.glTexImage2D(GLES20.GL_TEXTURE_2D, 0, glInfo.internalFormat, mSize.width,
> -                                mSize.height, 0, glInfo.format, glInfo.type, tempBuffer);
> -            GeckoAppShell.freeDirectBuffer(tempBuffer);
> -            */

Unrelated removal, but ok :)
Attachment #622372 - Flags: review?(chrislord.net) → review+
(In reply to Chris Lord [:cwiiis] from comment #1)
> 
> Unrelated removal, but ok :)

Showed up in my grepping for buffer-related code :)

Also, sewardj ran valgrind on fennec with this patch applied, and found no real difference in behaviour. He did see a crash on shutdown but turns out that was pre-existing; it is tracked by bug 753665.

Although it seems like this patch doesn't actually fix the memory leak valgrind was possibly finding, it seems like an improvement in the code so I landed it on inbound anyway.

https://hg.mozilla.org/integration/mozilla-inbound/rev/1259212fa0f7
status-firefox15: --- → fixed
Target Milestone: --- → Firefox 15
https://hg.mozilla.org/mozilla-central/rev/1259212fa0f7
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Marking this as wontfix for 14 because I'm not convinced it actually helped anything, and without being convinced of that it's not worth the risk.
status-firefox14: --- → wontfix
You need to log in before you can comment on or make changes to this bug.