Closed Bug 793904 Opened 12 years ago Closed 12 years ago

Intentionally crash if JS_TransplantObject fails

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: billm, Assigned: billm)

Details

Attachments

(1 file)

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.
Attached patch patch (deleted) — Splinter Review
This was green on try.
Attachment #664717 - Flags: review?(bobbyholley+bmo)
Attachment #664717 - Flags: review?(bobbyholley+bmo) → review+
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: