Closed Bug 895220 Opened 6 years ago Closed 6 years ago
New rooting hazards from ~Auto
GC Function: int32 JSObject::getElementNoGC(JSContext*, JSObject*, JSObject*, uint32, JS::Value*) uint8 js::IndexToIdNoGC(JSContext*, uint32, jsid*) uint8 js::IndexToIdSlow(js::ExclusiveContext*, uint32, class js::FakeMutableHandle<jsid>) [with js::AllowGC allowGC = (js::AllowGC)0u; uint32_t = unsigned int; typename js::MaybeRooted<jsid, allowGC>::MutableHandleType = js::FakeMutableHandle<jsid>] JSAtom* js::AtomizeChars(js::ExclusiveContext*, uint16*, uint64, uint32) [with js::AllowGC allowGC = (js::AllowGC)0u; jschar = short unsigned int; size_t = long unsigned int] jsatom.cpp:JSAtom* AtomizeAndCopyChars(js::ExclusiveContext*, uint16*, uint64, uint32) [with js::AllowGC allowGC = (js::AllowGC)0u; jschar = short unsigned int; size_t = long unsigned int] void js::AutoCompartment::~AutoCompartment(int32) void JSContext::leaveCompartment(JSCompartment*) void JSContext::wrapPendingException() uint8 JSCompartment::wrap(JSContext*, class JS::MutableHandle<JS::Value>, class JS::Handle<JSObject*>) FieldCall: JSRuntime.preWrapObjectCallback This is a result of the change: - AutoEnterAtomsCompartment ac(cx); + AutoCompartment ac(cx, cx->runtime()->atomsCompartment); AutoEnterAtomsCompartment just called cx->privateSetCompartment(oldCompartment) in its destructor, ignoring any pending exception. ~AutoCompartment wraps a pending exception. An exception is clearly possible, since AtomizeAndCopyChars can directly call js_ReportOutOfMemory, which calls cx->setPendingException(), so it seems like it was just wrong already. The question is, what should be done in NoGC code in this case? Should a new ~AutoCompartment<NoGC> just clear the pending exception, under the assumption that the user will be returning false/NULL in that case anyway, so the operation will be retried? I'm not familiar with the semantics of this NoGC stuff.
Last night I was not able to think of anything better than templatizing AtomizeAndCopyChars on AllowGC and making a version of it that returns false instead of OOM.
Actually, it already is templatized on AllowGC, so I think this is just a bug. Here is a one-line "fix".
Assignee: general → terrence
Status: NEW → ASSIGNED
Attachment #777904 - Flags: review?(bhackett1024)
Attachment #777904 - Flags: review?(bhackett1024) → review+
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.