Closed
Bug 944675
Opened 11 years ago
Closed 11 years ago
Fix reported rooting hazard in WebGLContextGL.cpp
Categories
(Core :: Graphics: CanvasWebGL, defect)
Core
Graphics: CanvasWebGL
Tracking
()
RESOLVED
FIXED
mozilla28
People
(Reporter: jonco, Assigned: jonco)
References
Details
(Whiteboard: [qa-])
Attachments
(1 file)
(deleted),
patch
|
bjacob
:
review+
|
Details | Diff | Splinter Review |
The rooting analysis reports the following hazards:
Function 'JS::Value mozilla::WebGLContext::GetUniform(JSContext*, mozilla::WebGLProgram*, mozilla::WebGLUniformLocation*)' has unrooted 'obj' of type 'JSObject*' live across GC call 'void mozilla::WebGLContext::ErrorOutOfMemory(int8*)' at content/canvas/src/WebGLContextGL.cpp:1904
Function 'JS::Value mozilla::WebGLContext::GetUniform(JSContext*, mozilla::WebGLProgram*, mozilla::WebGLUniformLocation*)' has unrooted 'obj:1' of type 'JSObject*' live across GC call 'void mozilla::WebGLContext::ErrorOutOfMemory(int8*)' at content/canvas/src/WebGLContextGL.cpp:1916
Function 'JS::Value mozilla::WebGLContext::GetUniform(JSContext*, mozilla::WebGLProgram*, mozilla::WebGLUniformLocation*)' has unrooted 'obj:2' of type 'JSObject*' live across GC call 'void mozilla::WebGLContext::ErrorOutOfMemory(int8*)' at content/canvas/src/WebGLContextGL.cpp:1931
The simplest fix for these is to return JS::NullValue() rather than touching the obj pointer after a potential GC.
Attachment #8340324 -
Flags: review?(bjacob)
Comment 1•11 years ago
|
||
I don't understand how this patch makes any difference?
JSObject* obj = Float32Array::Create(cx, this, unitSize, fv);
if (!obj) {
ErrorOutOfMemory("getUniform: out of memory");
+ return JS::NullValue();
}
return JS::ObjectOrNullValue(obj);
Here, we are in a if (!obj) branch, so the "return JS::NullValue();" is equivalent to the "return JS::ObjectOrNullValue(obj);" which we are doing at the next line.
Please explain!
Flags: needinfo?(jcoppeard)
Assignee | ||
Comment 2•11 years ago
|
||
The rooting analysis sees that the expression JS::ObjectOrNullValue(obj) makes use of an unrooted pointer (obj) after a possible GC in ErrorOutOfMemory(), so it reports this as a hazard.
It's not actually a hazard because we can see that ErrorOutOfMemory() is only called if obj is null, but making the analysis understand that in the general case would be very difficult.
Flags: needinfo?(jcoppeard)
Comment 3•11 years ago
|
||
Hm, wait, are you saying that ErrorOutOfMemory can trigger a GC? All it does, that is related to JS, is generate a JS warning.
That would indeed make it reasonable to want to avoid passing a JSObject* around across it.
Updated•11 years ago
|
Attachment #8340324 -
Flags: review?(bjacob) → review+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 4•11 years ago
|
||
Keywords: checkin-needed
Comment 5•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Updated•11 years ago
|
Whiteboard: [qa-]
You need to log in
before you can comment on or make changes to this bug.
Description
•