The default bug view has changed. See this FAQ.

Crash with memory-pressure, WebGL bufferData

RESOLVED FIXED in Firefox 15

Status

()

Core
Canvas: WebGL
--
critical
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Jesse Ruderman, Assigned: bjacob)

Tracking

(Blocks: 2 bugs, {crash, testcase})

Trunk
mozilla17
crash, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox15 verified, firefox16 fixed)

Details

(crash signature)

Attachments

(3 attachments)

(Reporter)

Description

5 years ago
Created attachment 639813 [details]
testcase (requires extension & pref)

1. Install https://www.squarefree.com/extensions/domFuzzLite3.xpi
2. Set 
     user_pref("dom.experimental_bindings", false);
3. Load the testcase.

Result: Crash [@ mozilla::WebGLContext::ErrorInvalidValue ]

Related to bug 764254 and/or bug 743753?
(Reporter)

Updated

5 years ago
Crash Signature: [@ mozilla::WebGLContext::ErrorInvalidValue]
(Reporter)

Comment 1

5 years ago
Created attachment 639817 [details]
stack trace

Nightly: bp-10007a87-6a62-42a7-8fac-0698c2120706
(Assignee)

Comment 2

5 years ago
This doesn't seem related to bug 764254. It is true that this wouldn't have happened before bug 743753 landed, but that is not the underlying problem. The problem seems that ErrorInvalidValue doesn't seem to behave gracefully under memory pressure. It should, as there is no reason to it to want to allocate much memory.
(Reporter)

Comment 3

5 years ago
"memory-pressure" is a notification that triggers GC, CC, and flushes caches. In this case, it's unrelated to actually being low on memory.
(Assignee)

Comment 4

5 years ago
Ah, that memory-pressure. OK, so memory-pressure causes the WebGL context to get 'lost' which means that all GL resources are freed. So I take back what I said in comment 2, this could well be the same issue as bug 764254, a general issue about WebGL contexts in lost state not behaving correctly.
(Assignee)

Comment 5

5 years ago
OK, and the stack trace you attached shows what's happening, the GL context pointer is reset to NULL and we're dereferencing that when we attempt to update the GL error state.

Bug 764254 is a similar but distinct bug, where we are also dereferencing this null gl context pointer.
(Assignee)

Comment 6

5 years ago
Created attachment 639934 [details] [diff] [review]
check for lost context in this overload of BufferData

Indeed, this overload of BufferData wasn't properly checking for context loss like other overloads and other webgl functions do.
Attachment #639934 - Flags: review?(jgilbert)

Comment 7

5 years ago
On Windows: bp-0c1bd681-bc4d-4f64-8054-972b92120707.
Crash Signature: [@ mozilla::WebGLContext::ErrorInvalidValue] → [@ mozilla::WebGLContext::ErrorInvalidValue] [@ mozilla::WebGLContext::MakeContextCurrent()]
OS: Mac OS X → All
Hardware: x86_64 → All
(Reporter)

Comment 8

5 years ago
Hmm, does sending a memory-pressure notification break all existing WebGL contexts?  That seems excessive compared to what memory-pressure does in the rest of Gecko.

(Indeed, pressing the "Minimize memory usage" button in about:memory causes the visualization on http://cubicvr.org/CubicVR.js/bd3/BeatDetektor3HD.html to go away.)
(Assignee)

Comment 9

5 years ago
Yes and no.

memory-pressure causes all WebGLContext to be "lost" immediately. However, WebGL applications then receive a webglcontextlost event that they are supposed to handle to request getting restored, in which case they receive a webglcontextrestored event which they are supposed to handle to restore their WebGL state.

Here is an example of a WebGL page that handles this correctly:
https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/demos/google/san-angeles/index.html

In practice, of course many WebGL pages do not handle this at all. This is a problem; having the "Minimize memory usage" button in about:memory causing WebGL context loss was considered an acceptable way of raising developer attention about this issue and providing an easy way to test any application. This can also be tested programatically using the WEBGL_lose_context extension.

Notice that context loss preexists WebGL. The OpenGL ES API that WebGL is based on, and that WebGL implementations build on, already have context loss. GPU resources are inherently non-permanent in many architectures (e.g. wrt device sleep).
(Reporter)

Comment 10

5 years ago
I see.  Thanks for explaining.
Attachment #639934 - Flags: review?(jgilbert) → review+
Oops, forgot to land.
https://tbpl.mozilla.org/?tree=Try&rev=237a172fc1c4
syntax error.
https://tbpl.mozilla.org/?tree=Try&rev=a2a80830d77c
http://hg.mozilla.org/integration/mozilla-inbound/rev/573c09668364
Assignee: nobody → bjacob
Target Milestone: --- → mozilla17
Comment on attachment 639934 [details] [diff] [review]
check for lost context in this overload of BufferData

[Approval Request Comment]
Bug caused by (feature/regressing bug #): since the switch of WebGLContext to new bindings, firefox 15
User impact if declined: occasional, non-security-sensitive webgl crash
Testing completed (on m-c, etc.): m-i
Risk to taking this patch (and alternatives if risky): no risk
String or UUID changes made by this patch: none
Attachment #639934 - Flags: approval-mozilla-beta?
Attachment #639934 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/573c09668364
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Comment on attachment 639934 [details] [diff] [review]
check for lost context in this overload of BufferData

[Triage Comment]
It's hard to argue with a no-risk crash fix for Aurora/Beta this early in the cycle. Approved.
Attachment #639934 - Flags: approval-mozilla-beta?
Attachment #639934 - Flags: approval-mozilla-beta+
Attachment #639934 - Flags: approval-mozilla-aurora?
Attachment #639934 - Flags: approval-mozilla-aurora+
http://hg.mozilla.org/releases/mozilla-aurora/rev/da26c9c61b2e
http://hg.mozilla.org/releases/mozilla-beta/rev/a6fba0be4e2d

Sorry for the delayed landing.

Updated

5 years ago
status-firefox15: --- → fixed
status-firefox16: --- → fixed
Verified using the steps to reproduce from the Description that Firefox 15 beta 4 does not crash when loading the attached test case. Verified on Windows XP, Ubuntu 12.04 and Mac OS X 10.6:
Mozilla/5.0 (Windows NT 5.1; rv:15.0) Gecko/20100101 Firefox/15.0
Mozilla/5.0 (X11; Linux i686; rv:15.0) Gecko/20100101 Firefox/15.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:15.0) Gecko/20100101 Firefox/15.0

Verified also the Socorro reports, and I didn't fond any crashes after this fix was pushed.
status-firefox15: fixed → verified
You need to log in before you can comment on or make changes to this bug.