Intentionally crash if JS_TransplantObject fails

RESOLVED FIXED in mozilla18

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: billm, Assigned: billm)

Tracking

unspecified
mozilla18
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

5.68 KB, patch
Bobby Holley (parental leave - send mail for anything urgent)
: review+
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
If we have any failures during the RemapAllWrappersForObject, we just return immediately. However, the heap will be all messed up at this point. We don't make any effort to "roll back", and doing so would be pretty difficult. I think it would make sense to crash immediately in this situation.

However, I'm not sure what the legitimate failure cases are during wrapping. The obvious one is OOM, and I think it's okay to crash there. The rest of the browser already does it. There also use to be some failure cases related to E4X wrapping, but that's gone now. We also have a recursion check during wrapping that could fail. I'm not sure if that's something that a user could trigger, though, especially once bug 787856 lands.

I haven't tracked down all the failure cases in XPConnect. For example, I don't know how the PreCreate hooks could fail. Bobby, what do you think about this?
I think we should definitely do this.

PreCreate hooks are a wild west of crazy stuff. I don't know of any of them that currently fail, but they're too numerous to audit easily. Let's just make this change, since it puts us in a definitively safer place than we are right now.
(Assignee)

Comment 2

5 years ago
Created attachment 664717 [details] [diff] [review]
patch

This was green on try.
Attachment #664717 - Flags: review?(bobbyholley+bmo)
Attachment #664717 - Flags: review?(bobbyholley+bmo) → review+
(Assignee)

Comment 3

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/691ed91f9413

Comment 4

5 years ago
https://hg.mozilla.org/mozilla-central/rev/691ed91f9413
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.