Ensure that iterators are closed when an exception is thrown (bug 729797, r=luke).
authorDavid Anderson <danderson@mozilla.com>
Mon, 27 Feb 2012 17:01:52 -0800
changeset 87890 aec23eb06a3e8b1b194b05a970e8e813ba6adf05
parent 87889 269bd7e1444ea3b93b836f08b42bae7416e111dc
child 87891 52b5e922f56e627b2df1fc1e7a92e85585dafa35
push id22160
push usermbrubeck@mozilla.com
push dateTue, 28 Feb 2012 17:21:33 +0000
treeherdermozilla-central@dde4e0089a18 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs729797
milestone13.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Ensure that iterators are closed when an exception is thrown (bug 729797, r=luke).
js/src/jsinterp.cpp
js/src/jsiter.cpp
js/src/jsiter.h
js/src/jswrapper.cpp
js/src/methodjit/InvokeHelpers.cpp
js/src/methodjit/StubCalls.cpp
--- a/js/src/jsinterp.cpp
+++ b/js/src/jsinterp.cpp
@@ -1962,17 +1962,17 @@ BEGIN_CASE(JSOP_IN)
     regs.sp[-1].setBoolean(cond);
 }
 END_CASE(JSOP_IN)
 
 BEGIN_CASE(JSOP_ITER)
 {
     JS_ASSERT(regs.sp > regs.fp()->base());
     uint8_t flags = GET_UINT8(regs.pc);
-    if (!js_ValueToIterator(cx, flags, &regs.sp[-1]))
+    if (!ValueToIterator(cx, flags, &regs.sp[-1]))
         goto error;
     CHECK_INTERRUPT_HANDLER();
     JS_ASSERT(!regs.sp[-1].isPrimitive());
 }
 END_CASE(JSOP_ITER)
 
 BEGIN_CASE(JSOP_MOREITER)
 {
@@ -1996,17 +1996,17 @@ BEGIN_CASE(JSOP_ITERNEXT)
     if (!IteratorNext(cx, &itervp->toObject(), &regs.sp[-1]))
         goto error;
 }
 END_CASE(JSOP_ITERNEXT)
 
 BEGIN_CASE(JSOP_ENDITER)
 {
     JS_ASSERT(regs.sp - 1 >= regs.fp()->base());
-    bool ok = !!js_CloseIterator(cx, &regs.sp[-1].toObject());
+    bool ok = CloseIterator(cx, &regs.sp[-1].toObject());
     regs.sp--;
     if (!ok)
         goto error;
 }
 END_CASE(JSOP_ENDITER)
 
 BEGIN_CASE(JSOP_DUP)
 {
@@ -4325,23 +4325,20 @@ END_CASE(JSOP_ARRAYPUSH)
                 PUSH_COPY(cx->getPendingException());
                 cx->clearPendingException();
                 len = 0;
                 DO_NEXT_OP(len);
 
               case JSTRY_ITER: {
                 /* This is similar to JSOP_ENDITER in the interpreter loop. */
                 JS_ASSERT(JSOp(*regs.pc) == JSOP_ENDITER);
-                Value v = cx->getPendingException();
-                cx->clearPendingException();
-                bool ok = js_CloseIterator(cx, &regs.sp[-1].toObject());
+                bool ok = UnwindIteratorForException(cx, &regs.sp[-1].toObject());
                 regs.sp -= 1;
                 if (!ok)
                     goto error;
-                cx->setPendingException(v);
               }
            }
         } while (++tn != tnlimit);
 
       no_catch:
         /*
          * Propagate the exception or error to the caller unless the exception
          * is an asynchronous return from a generator.
--- a/js/src/jsiter.cpp
+++ b/js/src/jsiter.cpp
@@ -824,17 +824,17 @@ iterator_iterator(JSContext *cx, JSObjec
 
 static JSBool
 Iterator(JSContext *cx, uintN argc, Value *vp)
 {
     Value *argv = JS_ARGV(cx, vp);
     bool keyonly = argc >= 2 ? js_ValueToBoolean(argv[1]) : false;
     uintN flags = JSITER_OWNONLY | (keyonly ? 0 : (JSITER_FOREACH | JSITER_KEYVALUE));
     *vp = argc >= 1 ? argv[0] : UndefinedValue();
-    return js_ValueToIterator(cx, flags, vp);
+    return ValueToIterator(cx, flags, vp);
 }
 
 JSBool
 js_ThrowStopIteration(JSContext *cx)
 {
     Value v;
 
     JS_ASSERT(!JS_IsExceptionPending(cx));
@@ -866,22 +866,29 @@ iterator_next(JSContext *cx, uintN argc,
 
 #define JSPROP_ROPERM   (JSPROP_READONLY | JSPROP_PERMANENT)
 
 static JSFunctionSpec iterator_methods[] = {
     JS_FN(js_next_str,      iterator_next,  0,JSPROP_ROPERM),
     JS_FS_END
 };
 
+#if JS_HAS_GENERATORS
+static JSBool
+CloseGenerator(JSContext *cx, JSObject *genobj);
+#endif
+
+namespace js {
+
 /*
  * Call ToObject(v).__iterator__(keyonly) if ToObject(v).__iterator__ exists.
  * Otherwise construct the default iterator.
  */
-JS_FRIEND_API(JSBool)
-js_ValueToIterator(JSContext *cx, uintN flags, Value *vp)
+JSBool
+ValueToIterator(JSContext *cx, uintN flags, Value *vp)
 {
     /* JSITER_KEYVALUE must always come with JSITER_FOREACH */
     JS_ASSERT_IF(flags & JSITER_KEYVALUE, flags & JSITER_FOREACH);
 
     /*
      * Make sure the more/next state machine doesn't get stuck. A value might be
      * left in iterValue when a trace is left due to an operation time-out after
      * JSOP_MOREITER but before the value is picked up by FOR*.
@@ -909,23 +916,18 @@ js_ValueToIterator(JSContext *cx, uintN 
             if (!obj)
                 return false;
         }
     }
 
     return GetIterator(cx, obj, flags, vp);
 }
 
-#if JS_HAS_GENERATORS
-static JSBool
-CloseGenerator(JSContext *cx, JSObject *genobj);
-#endif
-
-JS_FRIEND_API(JSBool)
-js_CloseIterator(JSContext *cx, JSObject *obj)
+bool
+CloseIterator(JSContext *cx, JSObject *obj)
 {
     cx->iterValue.setMagic(JS_NO_ITER_VALUE);
 
     if (obj->isIterator()) {
         /* Remove enumerators from the active list, which is a stack. */
         NativeIterator *ni = obj->getNativeIterator();
 
         if (ni->flags & JSITER_ENUMERATE) {
@@ -945,16 +947,29 @@ js_CloseIterator(JSContext *cx, JSObject
 #if JS_HAS_GENERATORS
     else if (obj->isGenerator()) {
         return CloseGenerator(cx, obj);
     }
 #endif
     return JS_TRUE;
 }
 
+bool
+UnwindIteratorForException(JSContext *cx, JSObject *obj)
+{
+    Value v = cx->getPendingException();
+    cx->clearPendingException();
+    if (!CloseIterator(cx, obj))
+        return false;
+    cx->setPendingException(v);
+    return true;
+}
+
+} // namespace js
+
 /*
  * Suppress enumeration of deleted properties. This function must be called
  * when a property is deleted and there might be active enumerators.
  *
  * We maintain a list of active non-escaping for-in enumerators. To suppress
  * a property, we check whether each active enumerator contains the (obj, id)
  * pair and has not yet enumerated |id|. If so, and |id| is the next property,
  * we simply advance the cursor. Otherwise, we delete |id| from the list.
--- a/js/src/jsiter.h
+++ b/js/src/jsiter.h
@@ -176,29 +176,32 @@ VectorToValueIterator(JSContext *cx, JSO
 
 /*
  * Creates either a key or value iterator, depending on flags. For a value
  * iterator, performs value-lookup to convert the given list of jsids.
  */
 bool
 EnumeratedIdVectorToIterator(JSContext *cx, JSObject *obj, uintN flags, js::AutoIdVector &props, js::Value *vp);
 
-}
-
 /*
  * Convert the value stored in *vp to its iteration object. The flags should
  * contain JSITER_ENUMERATE if js_ValueToIterator is called when enumerating
  * for-in semantics are required, and when the caller can guarantee that the
  * iterator will never be exposed to scripts.
  */
-extern JS_FRIEND_API(JSBool)
-js_ValueToIterator(JSContext *cx, uintN flags, js::Value *vp);
+extern JSBool
+ValueToIterator(JSContext *cx, uintN flags, js::Value *vp);
 
-extern JS_FRIEND_API(JSBool)
-js_CloseIterator(JSContext *cx, JSObject *iterObj);
+extern bool
+CloseIterator(JSContext *cx, JSObject *iterObj);
+
+extern bool
+UnwindIteratorForException(JSContext *cx, JSObject *obj);
+
+}
 
 extern bool
 js_SuppressDeletedProperty(JSContext *cx, JSObject *obj, jsid id);
 
 extern bool
 js_SuppressDeletedElement(JSContext *cx, JSObject *obj, uint32_t index);
 
 extern bool
--- a/js/src/jswrapper.cpp
+++ b/js/src/jswrapper.cpp
@@ -651,17 +651,17 @@ CanReify(Value *vp)
            (obj = &vp->toObject())->getClass() == &IteratorClass &&
            (obj->getNativeIterator()->flags & JSITER_ENUMERATE);
 }
 
 struct AutoCloseIterator
 {
     AutoCloseIterator(JSContext *cx, JSObject *obj) : cx(cx), obj(obj) {}
 
-    ~AutoCloseIterator() { if (obj) js_CloseIterator(cx, obj); }
+    ~AutoCloseIterator() { if (obj) CloseIterator(cx, obj); }
 
     void clear() { obj = NULL; }
 
   private:
     JSContext *cx;
     JSObject *obj;
 };
 
@@ -696,17 +696,17 @@ Reify(JSContext *cx, JSCompartment *orig
             id = js_CheckForStringIndex(id);
             keys[i] = id;
             if (!origin->wrapId(cx, &keys[i]))
                 return false;
         }
     }
 
     close.clear();
-    if (!js_CloseIterator(cx, iterObj))
+    if (!CloseIterator(cx, iterObj))
         return false;
 
     if (isKeyIter)
         return VectorToKeyIterator(cx, obj, ni->flags, keys, vp);
     return VectorToValueIterator(cx, obj, ni->flags, keys, vp); 
 }
 
 bool
--- a/js/src/methodjit/InvokeHelpers.cpp
+++ b/js/src/methodjit/InvokeHelpers.cpp
@@ -146,24 +146,21 @@ top:
                 {
                   /*
                    * This is similar to JSOP_ENDITER in the interpreter loop,
                    * except the code now uses the stack slot normally used by
                    * JSOP_NEXTITER, namely regs.sp[-1] before the regs.sp -= 2
                    * adjustment and regs.sp[1] after, to save and restore the
                    * pending exception.
                    */
-                  Value v = cx->getPendingException();
                   JS_ASSERT(JSOp(*pc) == JSOP_ENDITER);
-                  cx->clearPendingException();
-                  bool ok = !!js_CloseIterator(cx, &cx->regs().sp[-1].toObject());
+                  bool ok = UnwindIteratorForException(cx, &cx->regs().sp[-1].toObject());
                   cx->regs().sp -= 1;
                   if (!ok)
                       goto top;
-                  cx->setPendingException(v);
                 }
             }
         }
     }
 
     return NULL;
 }
 
--- a/js/src/methodjit/StubCalls.cpp
+++ b/js/src/methodjit/StubCalls.cpp
@@ -1225,17 +1225,17 @@ stubs::GetPropNoCache(VMFrame &f, Proper
         THROW();
 
     regs.sp[-1] = rval;
 }
 
 void JS_FASTCALL
 stubs::Iter(VMFrame &f, uint32_t flags)
 {
-    if (!js_ValueToIterator(f.cx, flags, &f.regs.sp[-1]))
+    if (!ValueToIterator(f.cx, flags, &f.regs.sp[-1]))
         THROW();
     JS_ASSERT(!f.regs.sp[-1].isPrimitive());
 }
 
 static void
 InitPropOrMethod(VMFrame &f, PropertyName *name, JSOp op)
 {
     JSContext *cx = f.cx;
@@ -1300,17 +1300,17 @@ stubs::IterMore(VMFrame &f)
 
     return v.toBoolean();
 }
 
 void JS_FASTCALL
 stubs::EndIter(VMFrame &f)
 {
     JS_ASSERT(f.regs.sp - 1 >= f.fp()->base());
-    if (!js_CloseIterator(f.cx, &f.regs.sp[-1].toObject()))
+    if (!CloseIterator(f.cx, &f.regs.sp[-1].toObject()))
         THROW();
 }
 
 JSString * JS_FASTCALL
 stubs::TypeOf(VMFrame &f)
 {
     const Value &ref = f.regs.sp[-1];
     JSType type = JS_TypeOfValue(f.cx, ref);