941800 - crash in mozilla::gl::SwapRAndBComponents

Status: RESOLVED INCOMPLETE

(Core :: Graphics, defect)

Description

[Brad Lassey [:blassey] (use needinfo?)]

This bug was filed from the Socorro interface and is 
report bp-71c4506a-cd19-4bed-abdd-60aeb2131119.
=============================================================

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

Reliable test case at http://gojs.net/beta/tests/

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

The test that crashes is "TestRoot|GridLayout|Alignment|Location, varied sizes"

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

This is an interesting bug. The canvas here is pretty large (676x1192). When are drawing the first frame via SkiaGL, SharedSurfaceEGL needs to do a readback (due to reasons I don't fully understand). We fail to allocate the memory for that buffer, but never check that, so we crash in a null deref. We can fix the crash, of course, but we'll still end up with a blank frame here. Looking a little deeper, but will also post a patch to avoid the crash.

Comment 5

[James Willcox (:snorp) ([e-mail]) (he/him)]

For the URL in question, fixing the crash here just causes it to crash later on somewhere else. Firefox is using 550MB RSS at the time of the crash, though, so probably about to be OOM killed anyway.

Comment 6

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Make sure we have a valid buffer before reading pixels

Comment 7

[Simon Sarris]

Thanks for your investigation by the way. You guys have always been so much better at being responsive and investigative than the other browser vendors I report bugs with.

Comment 8

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Simon Sarris from comment #7)
> Thanks for your investigation by the way. You guys have always been so much
> better at being responsive and investigative than the other browser vendors
> I report bugs with.

Thanks for reporting! Sorry it doesn't look like we're going to be able to make your tests pass (at least not with the patch here). Is it possible to change your test harness to use less memory? Are you keeping around a bunch of canvases or something?

Comment 9

[Simon Sarris]

Thanks, I'll look into the OOM stuff. I probably am piling something up.

Comment 10

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8382448 [details] [diff] [review]
Make sure we have a valid buffer before reading pixels

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

Cool. I'm always a fan of proper error handling.
We'll still need callers to be updated to handle this failing, though.

Comment 11

[Rob]

(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #0)
> This bug was filed from the Socorro interface and is 
> report bp-71c4506a-cd19-4bed-abdd-60aeb2131119.
> =============================================================

[@ mozilla::gl::SwapRAndBComponents ] - Socorro says "Related Bugs 941800".
bp-af01531a-fcf8-432b-9c4e-e987a2140317	3/16/20 - Caused by "Google Street View", in Firefox Nightly, under WinXP.

.

I also crashed while using "GSV" and caused these ('not', according to Socorro) Related Bugs:

[@ mozalloc_abort(char const* const) | NS_DebugBreak | mozilla::ipc::FatalError(char const*, char const*, void*, bool) ]
bp-bc271d62-8b0c-41fb-9e5f-08d0d2140317

[@ rx::Renderer9::readPixels(gl::Framebuffer*, int, int, int, int, unsigned int, unsigned int, int, bool, int, void*) ]
bp-f035df44-fe94-4b5f-8769-916662140317

[@ mozalloc_abort(char const* const) | mozalloc_handle_oom(unsigned int) | moz_xrealloc | nsStreamLoader::WriteSegmentFun(nsIInputStream*, void*, char const*, unsigned int, unsigned int, unsigned int*) ]
bp-bc271d62-8b0c-41fb-9e5f-08d0d2140317


The last one is me (or the Browser's inability to allocate Virtual Swap Memeory under Win XP) running out of Memory and is definitely not related. 

I only included the last few as all the above Crashes are ALL my Crashes using "GSV" (today). Prior to today "GSV" did not crash the Browser on me. Recently "GSV" is a good way to generate a crash.

Comment 12

[Rob]

Aurora bp-4f8266a3-26ed-4284-9d0c-d77f72140321 .

Comment 13

[Simon Sarris]

This is no longer crashing in my original test-case. Is the underlying issue discovered here all set?

Comment 14

[Kelsey Gilbert [:jgilbert]]

Is this ready to land?

Comment 15

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Jeff Gilbert [:jgilbert] from comment #14)
> Is this ready to land?

That patch didn't really fix anything, IIRC, it was just speculation.

Comment 16

[u279076]

There are no reports of this crash beyond Fennec 30. Please reopen this bug report if you can reproduce the crash in a current version.