Closed Bug 791521 Opened 7 years ago Closed 7 years ago

crash in mozilla::WebGLContext::LoseOldestWebGLContextIfLimitExceeded


(Core :: Canvas: WebGL, defect, critical)

17 Branch
Not set





(Reporter: scoobidiver, Assigned: bjacob)



(Keywords: crash, regression, reproducible, Whiteboard: [native-crash])

Crash Data


(1 file)

There's one crash in 18.0a1/20120915: bp-d6bf337a-21b8-4f4a-a78a-9b1d62120916.

Signature 	mozilla::WebGLContext::LoseOldestWebGLContextIfLimitExceeded More Reports Search
UUID	d6bf337a-21b8-4f4a-a78a-9b1d62120916
Date Processed	2012-09-16 05:26:26
Uptime	45
Last Crash	30.1 minutes before submission
Install Age	3.0 hours since version was first installed.
Install Time	2012-09-16 02:25:57
Product	FennecAndroid
Version	18.0a1
Build ID	20120915030531
Release Channel	nightly
OS	Linux
OS Version	0.0.0 Linux #1 SMP PREEMPT Sun Sep 9 22:20:24 EST 2012 armv7l
Build Architecture	arm
Build Architecture Info	
Crash Reason	SIGSEGV
Crash Address	0xc
App Notes 	
AdapterDescription: 'NVIDIA Corporation -- NVIDIA Tegra -- OpenGL ES 2.0 -- Model: MZ601, Product: RTCOREEU, Manufacturer: motorola, Hardware: stingray'
EGL? EGL+ GL Context? GL Context+ GL Layers? GL Layers+ WebGL! WebGL+ 
motorola MZ601
EMCheckCompatibility	False
Adapter Vendor ID	NVIDIA Corporation
Adapter Device ID	NVIDIA Tegra
Accessibility	Active
Device	motorola MZ601
Android API Version	16 (REL)
Android CPU ABI	armeabi-v7a

Frame 	Module 	Signature 	Source
0 	mozilla::WebGLContext::LoseOldestWebGLContextIfLimitExceeded 	nsNodeInfoManager.h:94
1 	mozilla::WebGLContext::SetDimensions 	WebGLContext.cpp:383
2 	mozilla::widget::GfxInfoWebGL::GetWebGLParameter 	GfxInfoWebGL.cpp:41
3 	mozilla::widget::GfxInfoBase::GetWebGLParameter 	GfxInfoBase.cpp:728
4 	NS_InvokeByIndex_P 	xptcinvoke_arm.cpp:160
5 	XPCWrappedNative::CallMethod 	XPCWrappedNative.cpp:3105
6 	XPC_WN_CallMethod 	XPCWrappedNativeJSOps.cpp:1469
7 	js::InvokeKernel 	jscntxtinlines.h:372
8 	js::Interpret 	jsinterp.cpp:2454
9 	js::RunScript 	jsinterp.cpp:324
10 	js::Invoke 	jsinterp.cpp:378
11 	JS_CallFunctionValue 	jsapi.cpp:5906
12 	nsJSContext::CallEventHandler 	nsJSEnvironment.cpp:1926
13 	nsJSEventListener::HandleEvent 	nsJSEventListener.cpp:188
14 	nsEventListenerManager::HandleEventSubType 	nsEventListenerManager.cpp:846
15 	nsEventListenerManager::HandleEventInternal 	nsEventListenerManager.cpp:919
16 	nsEventTargetChainItem::HandleEvent 	nsEventListenerManager.h:144
17 	nsEventTargetChainItem::HandleEventTargetChain 	nsEventDispatcher.cpp:317
18 	nsEventDispatcher::Dispatch 	nsEventDispatcher.cpp:640
19 	DocumentViewerImpl::LoadComplete 	nsDocumentViewer.cpp:1027
20 	nsDocShell::EndPageLoad 	nsDocShell.cpp:6414 

More reports at:
This is a null ptr deref in DocumentPrincipal() called from LoseOldestWebGLContextIfLimitExceeded() which can only come from one of the NodePrincipal() calls made there, which has to mean that NodePrincipal() was called on a null object.

These calls are (at line 676):

        nsIPrincipal *ourPrincipal = GetCanvas()->NodePrincipal();
        nsIPrincipal *theirPrincipal = contexts[i]->GetCanvas()->NodePrincipal();

We already check above that contexts[i]->GetCanvas() is non-null. So the only explanation is that this->GetCanvas() is null. Since we were called by SetDimensions, this means that SetDimensions was called on a WebGLContext which has a null canvas element pointer.

I'm not sure whether that is a bug in itself or is legitimate so by default I'll assume it's legitimate and we want to handle this case. This patch checks for null canvas in SetDimensions and returns early.
Attachment #661591 - Flags: review?(jgilbert)
Comment on attachment 661591 [details] [diff] [review]
in SetDimensions return early if canvas is null

This could use some feedback as to whether it is legitimate that WebGLContext::SetDimensions is called on a WebGLContext that has a null mCanvasElement pointer, and whether silentlty returning early in this case is the right thing to do.
Attachment #661591 - Flags: feedback?(roc)
Comment on attachment 661591 [details] [diff] [review]
in SetDimensions return early if canvas is null

Review of attachment 661591 [details] [diff] [review]:

This patch is OK, but I can't think of any reason why GetCanvas() would return null for a WebGLContext. As far as I can tell we only clear mCanvasElement during cycle collection Unlink, and this crash stack is not happening during cycle collection. (Maybe it's possible that during cycle collection something resurrects the canvas by adding a reference to it from a live object. I have no idea how that could happen.)
Attachment #661591 - Flags: feedback?(roc) → feedback+
STR on desktop:
1. Load
2. Click Run
3. Open the Troubleshooting Information tab
=> bp-89cfe854-ff89-4c86-826f-273a62120917
Crash Signature: [@ mozilla::WebGLContext::LoseOldestWebGLContextIfLimitExceeded] → [@ mozilla::WebGLContext::LoseOldestWebGLContextIfLimitExceeded] [@ nsINode::NodePrincipal()]
Keywords: reproducible
OS: Android → All
Hardware: ARM → All
Keywords: regression
Version: Trunk → 17 Branch
Attachment #661591 - Flags: review?(jgilbert) → review+
It turns out, this patch caused a crash in mochitest-other. On null canvas, we should return NS_ERROR_FAILURE, not NS_OK. This fixes the crash.
Assignee: nobody → bjacob
Target Milestone: --- → mozilla18
Closed: 7 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Depends on: 795701
Comment on attachment 661591 [details] [diff] [review]
in SetDimensions return early if canvas is null

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined:  about:support crashes on certain drivers (at least Mesa)
Fix Landed on Version: 18
Risk to taking this patch (and alternatives if risky): this will regress about:support WebGL renderer, this field will become empty. I don't think that this matters much, as I don't expect that ESR users are very interested in WebGL and only a minority of those who do would notice that WebGL info is missing from about:support (WebGL itself keeps working)
String or UUID changes made by this patch: none

See for more info.
Attachment #661591 - Flags: approval-mozilla-esr17?
Comment on attachment 661591 [details] [diff] [review]
in SetDimensions return early if canvas is null

Not a particularly critical fix for ESR users. No need to uplift.
Attachment #661591 - Flags: approval-mozilla-esr17? → approval-mozilla-esr17-
You need to log in before you can comment on or make changes to this bug.