Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: bholley, Assigned: bholley)

Tracking

unspecified
mozilla23
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox22 fixed, firefox23 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

6 years ago
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.
> which can certainly OOM

But should it be able to do so non-fatally?

Updated

6 years ago
Blocks: 858648
Assignee

Comment 2

6 years ago
Attachment #733936 - Flags: review?(bzbarsky)
Assignee

Comment 3

6 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?
> 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

6 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 on attachment 733936 [details] [diff] [review]
Null-check the XBL scope. v1

r=me
Attachment #733936 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/b7f51945a2b7
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
Assignee

Comment 9

6 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?
Attachment #733936 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.