Closed Bug 1329113 Opened 4 years ago Closed 4 years ago

AutoCloseIterator::~AutoCloseIterator ignores the return value of CloseIterator


(Core :: JavaScript Engine, defect)

Not set



Tracking Status
firefox53 --- affected
firefox54 --- fixed


(Reporter: arai, Assigned: arai)



(1 file)
>     ~AutoCloseIterator() { if (obj) CloseIterator(cx, obj); }

If it never fails, it needs MOZ_ALWAYS_TRUE or something.
if it can fail, we should handle the case.
It can fail. CloseIterator can call LegacyGeneratorObject::close can call GlobalObject::getIntrinsicValue which can fail. It can probably fail when it Calls the LegacyGeneratorCloseInternal intrinsic, whatever that is.
> static bool
> Reify(JSContext* cx, JSCompartment* origin, MutableHandleObject objp)
> {
>     Rooted<PropertyIteratorObject*> iterObj(cx, &objp->as<PropertyIteratorObject>());
>     NativeIterator* ni = iterObj->getNativeIterator();
>     AutoCloseIterator close(cx, iterObj);

AutoCloseIterator is used only here, so `iterObj` is PropertyIteratorObject, and CloseIterator uses infallible path:
> bool
> js::CloseIterator(JSContext* cx, HandleObject obj)
> {
>     if (obj->is<PropertyIteratorObject>()) {
>         /* Remove enumerators from the active list, which is a stack. */
>         NativeIterator* ni = obj->as<PropertyIteratorObject>().getNativeIterator();
>         if (ni->flags & JSITER_ENUMERATE) {
>             ni->unlink();
>             MOZ_ASSERT(ni->flags & JSITER_ACTIVE);
>             ni->flags &= ~JSITER_ACTIVE;
>             /*
>              * Reset the enumerator; it may still be in the cached iterators
>              * for this thread, and can be reused.
>              */
>             ni->props_cursor = ni->props_array;
>         }

So, maybe we should just change AutoCloseIterator to receive PropertyIteratorObject*,
and then use MOZ_ALWAYS_TRUE for CloseIterator call.

so that the fallible path for LegacyGeneratorObject won't be taken.
Changed AutoCloseIterator to receive PropertyIteratorObject*, and put CloseIterator into MOZ_ALWAYS_TRUE, in AutoCloseIterator and Reify, where object is always PropertyIteratorObject.
Assignee: nobody → arai.unmht
Attachment #8841304 - Flags: review?(sphink)
Comment on attachment 8841304 [details] [diff] [review]
Use PropertyIteratorObject* in AutoCloseIterator.

Review of attachment 8841304 [details] [diff] [review]:

Oh, I see. That seems fine.
Attachment #8841304 - Flags: review?(sphink) → review+
The last inbound -> central merge didn't get the bugs marked as fixed (this one was the last bug from that merge)
Flags: needinfo?(cbook)
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
(In reply to Guilherme Lima from comment #6)
> The last inbound -> central merge didn't get the bugs marked as fixed (this
> one was the last bug from that merge)

Hi Guilherme yeah that was because there was a problem with treeherder and i waited till this was fixed. Should be ok now :)
Flags: needinfo?(cbook)
You need to log in before you can comment on or make changes to this bug.