Status

()

Firefox for Android
Toolbar
--
critical
4 years ago
2 years ago

People

(Reporter: mt, Assigned: gw280)

Tracking

(Depends on: 1 bug, {crash})

Trunk
ARM
Android
crash
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
This URL (warning, not friendly to eyes) reliably crashes on the latest android release.  It contains a large number of images, many of them animated gifs.  

Note that most of the images load slowly, and none contain width or height attributes.  That means almost all of them are in the viewport on initial render.  That might indicate resource exhaustion.

Tested on a Galaxy S4.
Not a security bug.
Group: core-security
I get a flood of 

W/Adreno-EGL(10407): <qeglDrvAPI_eglSwapBuffers:3692>: EGL_BAD_SURFACE
E/BufferQueue(  261): [SurfaceView] dequeueBuffer: can't dequeue multiple buffers without setting the buffer count

E/Surface (10407): dequeueBuffer: IGraphicBufferProducer::requestBuffer failed: 1717840517
E/Surface (10407): dequeueBuffer failed (Unknown error -1717840517)
E/Surface (10407): Surface::unlockAndPost failed, no locked buffer
E/ViewRootImpl(10407): Could not unlock surface
E/ViewRootImpl(10407): java.lang.IllegalArgumentException
E/ViewRootImpl(10407): 	at android.view.Surface.nativeUnlockCanvasAndPost(Native Method)

and then an eventual crash

I/ActivityManager(  793): Process org.mozilla.fennec (pid 10407) (adj 0) has died.
I/WindowState(  793): WIN DEATH: Window{42e7e318 u0 org.mozilla.fennec/org.mozilla.fennec.App}

with a signature

bp-5205c6cb-ccc9-4c9e-a663-e01cf2140711
Severity: normal → critical
Keywords: crash
Hardware: Other → ARM
Version: unspecified → Trunk

Updated

4 years ago
Component: General → Graphics, Panning and Zooming
Assignee: nobody → edwin
I can reproduce this locally. Will investigate.
Assignee: edwin → gwright
E/IMGSRV  ( 7082): :0: PrepareToDraw: Invalid drawable
E/IMGSRV  ( 7082): :0: glDrawArrays: Can't prepare to draw
E/IMGSRV  ( 7082): :0: WSEGL_GetDrawableParameters: Native buffer became NULL
E/IMGSRV  ( 7082): :0: KEGLGetDrawableParameters: Native window is invalid

Those four lines loop over and over again until Firefox crashes.
Created attachment 8759818 [details] [diff] [review]
0001-Bug-1037534-Ensure-SurfacePipeFactory-MakePipe-handl.patch

I don't think this is sufficient, but it's required.
Attachment #8759818 - Flags: review?(seth)
(In reply to George Wright (:gw280) (:gwright) from comment #5)
> Created attachment 8759818 [details] [diff] [review]
> 0001-Bug-1037534-Ensure-SurfacePipeFactory-MakePipe-handl.patch
> 
> I don't think this is sufficient, but it's required.

Uhhh, what? MakeUnique just calls new, and I don't see where fallible allocation is happening in that code path. Can you explain to me what I'm missing?
Attachment #8759818 - Flags: review?(seth)
So I've been mulling this over; Seth, you're right in that operator new is infallible. I put this check in and it appeared to stop my build/test from crashing, but I'm going to assume because it's OOM related it was random chance that it didn't crash.

I don't really see many good things we can do here. If we're crashing due to OOM because of this, changing the image to be allocated using a fallible allocator will only delay the inevitable as presumably we have no memory so we're screwed anyway. We could maybe switch it to a fallible allocator then raise a memory pressure event?

Alternatively, we can try and come up with a heuristic to determine how many images to load into memory. I don't know what this would look like in practice. I'd suspect something like a hard limit on the number of images and also keep track of how many bytes we've allocated for images and cap that based on system memory.

I don't really see any "good" solutions here. Maybe the best thing to do is just do nothing at all.
So looking at this from another angle, it appears to only crash using a locally-built debug build. Using a nightly build (or a local release build), it gets into a state where the browser UI is still responsive, but the browser is unusable (pages never load etc), and with the same looping messages from comment 4. I haven't been able to reproduce my initial assessment from comment 4 where it would crash afterwards though.
Spoke to Seth about this. Here's the gist of the issue:

01:39 < seth> gw280: we're hitting it on a page with a large number of animated GIFs
01:39 < seth> once we allocate memory for an animated GIF's frames, it stays locked as long as you remain on that page
01:39 < seth> gw280: this is probably the problem. it's *not* that we fail to allocate memory in imagelib. it's that the memory consumed by the GIFs is causing infallible allocation failure elsewhere
01:40 < seth> i'm literally working *right now* towards eliminating that problem with GIFs but it will take a while

Seth is working on this in bug 1257388
(Assignee)

Updated

2 years ago
Depends on: 1257388
You need to log in before you can comment on or make changes to this bug.