Closed
Bug 858642
Opened 12 years ago
Closed 12 years ago
Null-check the XBL scope
Categories
(Core :: XPConnect, defect)
Tracking
()
RESOLVED
FIXED
mozilla23
People
(Reporter: bholley, Assigned: bholley)
References
Details
Attachments
(1 file)
(deleted),
patch
|
bzbarsky
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
XBL scope creation is lazy, and involves allocating an entire compartment, which can certainly OOM. Some of the crashes in bug 854604 comment 6 appear to be related to the XBL scope being null.
It's tempting to just use the global of XBL scope creation fails, but I don't think we should do that. If we decide we ought to be using an XBL scope, we should just fail if we can't make that happen.
Comment 1•12 years ago
|
||
> which can certainly OOM
But should it be able to do so non-fatally?
Assignee | ||
Comment 2•12 years ago
|
||
Attachment #733936 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #1)
> But should it be able to do so non-fatally?
JS heap allocations are fallible, right? I mean, we could just MOZ_CRASH if that happens, but that doesn't solve the crashes here. Or maybe sandbox creation is failing for some other reason?
Comment 4•12 years ago
|
||
> I mean, we could just MOZ_CRASH if that happens
That's what I meant, but good point about people hitting that....
Assignee | ||
Comment 5•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #4)
> > I mean, we could just MOZ_CRASH if that happens
>
> That's what I meant, but good point about people hitting that....
Yeah. There's a chance that this is really some other issue in disguise, and that the fix here will just end up being wallpaper. The alternative is to make the sandbox scope allocation infallible and then see if we start seeing these crashes turn into allocation failure crashes, but that's a two-step process, which may involve backing out the first step. So more of a pain.
Comment 6•12 years ago
|
||
Comment on attachment 733936 [details] [diff] [review]
Null-check the XBL scope. v1
r=me
Attachment #733936 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 7•12 years ago
|
||
Comment 8•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Assignee | ||
Comment 9•12 years ago
|
||
Comment on attachment 733936 [details] [diff] [review]
Null-check the XBL scope. v1
[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 821850. That bug is now on beta, but it sounds like this may (or may not) fix the crashes in bug 858648. So I think we should land this on aurora now and see if it affects crash volume.
User impact if declined: Crashes, potentially.
Testing completed (on m-c, etc.): Baked on m-c.
Risk to taking this patch (and alternatives if risky): Not risky.
String or IDL/UUID changes made by this patch: None.
Attachment #733936 -
Flags: approval-mozilla-aurora?
Updated•12 years ago
|
Attachment #733936 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 10•12 years ago
|
||
status-firefox22:
--- → fixed
status-firefox23:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•