Suppress deleted properties during iteration (569735, r=brendan).

This commit is contained in:
Andreas Gal 2010-06-03 21:41:01 -07:00
parent 864fd5ca21
commit c3f36f58a1
9 changed files with 139 additions and 12 deletions

View File

@ -102,6 +102,7 @@
#include "jsfun.h"
#include "jsgc.h"
#include "jsinterp.h"
#include "jsiter.h"
#include "jslock.h"
#include "jsnum.h"
#include "jsobj.h"
@ -1051,6 +1052,9 @@ array_deleteProperty(JSContext *cx, JSObject *obj, jsval id, jsval *rval)
obj->setDenseArrayElement(i, JSVAL_HOLE);
}
if (!js_SuppressDeletedProperty(cx, obj, id))
return false;
*rval = JSVAL_TRUE;
return JS_TRUE;
}

View File

@ -1048,6 +1048,8 @@ js_DestroyContext(JSContext *cx, JSDestroyContextMode mode)
JSContextCallback cxCallback;
JSBool last;
JS_ASSERT(!cx->enumerators);
rt = cx->runtime;
#ifdef JS_THREADSAFE
/*

View File

@ -1853,6 +1853,9 @@ struct JSContext
DSTOffsetCache dstOffsetCache;
/* List of currently active non-escaping enumerators (for-in). */
JSObject *enumerators;
private:
/*
* To go from a live generator frame (on the stack) to its generator object

View File

@ -418,6 +418,21 @@ const uint16 js_PrimitiveTestFlags[] = {
JSFUN_THISP_NUMBER /* INT */
};
class AutoPreserveEnumerators {
JSContext *cx;
JSObject *enumerators;
public:
AutoPreserveEnumerators(JSContext *cx) : cx(cx), enumerators(cx->enumerators)
{
}
~AutoPreserveEnumerators()
{
cx->enumerators = enumerators;
}
};
/*
* Find a function reference and its 'this' object implicit first parameter
* under argc arguments on cx's stack, and call the function. Push missing
@ -645,6 +660,7 @@ js_Invoke(JSContext *cx, const InvokeArgsGuard &args, uintN flags)
#endif
} else {
JS_ASSERT(script);
AutoPreserveEnumerators preserve(cx);
ok = js_Interpret(cx);
}
@ -834,6 +850,7 @@ js_Execute(JSContext *cx, JSObject *const chain, JSScript *script,
if (JSInterpreterHook hook = cx->debugHooks->executeHook)
hookData = hook(cx, fp, JS_TRUE, 0, cx->debugHooks->executeHookData);
AutoPreserveEnumerators preserve(cx);
JSBool ok = js_Interpret(cx);
if (result)
*result = fp->rval;

View File

@ -107,6 +107,7 @@ NativeIterator::mark(JSTracer *trc)
JS_SET_TRACING_INDEX(trc, "props", (vp - props_array));
js_CallValueTracerIfGCThing(trc, *vp);
}
JS_CALL_OBJECT_TRACER(trc, obj, "obj");
}
/*
@ -230,13 +231,14 @@ EnumerateDenseArrayProperties(JSContext *cx, JSObject *obj, JSObject *pobj, uint
}
NativeIterator *
NativeIterator::allocate(JSContext *cx, uintN flags, uint32 *sarray, uint32 slength, uint32 key,
jsval *parray, uint32 plength)
NativeIterator::allocate(JSContext *cx, JSObject *obj, uintN flags, uint32 *sarray, uint32 slength,
uint32 key, jsval *parray, uint32 plength)
{
NativeIterator *ni = (NativeIterator *)
cx->malloc(sizeof(NativeIterator) + plength * sizeof(jsval) + slength * sizeof(uint32));
if (!ni)
return NULL;
ni->obj = obj;
ni->props_array = ni->props_cursor = (jsval *) (ni + 1);
ni->props_end = ni->props_array + plength;
if (plength)
@ -315,7 +317,7 @@ Snapshot(JSContext *cx, JSObject *obj, uintN flags, uint32 *sarray, uint32 sleng
pobj = pobj->getProto();
}
return NativeIterator::allocate(cx, flags, sarray, slength, key, props.begin(), props.length());
return NativeIterator::allocate(cx, obj, flags, sarray, slength, key, props.begin(), props.length());
}
bool
@ -399,8 +401,18 @@ NewIteratorObject(JSContext *cx, uintN flags)
: NewObjectWithGivenProto(cx, &js_IteratorClass.base, NULL, NULL);
}
static inline void
RegisterEnumerator(JSContext *cx, JSObject *iterobj, NativeIterator *ni)
{
/* Register non-escaping native enumerators (for-in) with the current context. */
if (ni->flags & JSITER_ENUMERATE) {
ni->next = cx->enumerators;
cx->enumerators = iterobj;
}
}
bool
JSIdArrayToIterator(JSContext *cx, uintN flags, JSIdArray *ida, jsval *vp)
JSIdArrayToIterator(JSContext *cx, JSObject *obj, uintN flags, JSIdArray *ida, jsval *vp)
{
JSObject *iterobj = NewIteratorObject(cx, flags);
if (!iterobj)
@ -408,12 +420,14 @@ JSIdArrayToIterator(JSContext *cx, uintN flags, JSIdArray *ida, jsval *vp)
*vp = OBJECT_TO_JSVAL(iterobj);
NativeIterator *ni = NativeIterator::allocate(cx, flags, NULL, 0, 0,
NativeIterator *ni = NativeIterator::allocate(cx, obj, flags, NULL, 0, 0,
ida->vector, ida->length);
if (!ni)
return false;
iterobj->setNativeIterator(ni);
RegisterEnumerator(cx, iterobj, ni);
return true;
}
@ -460,6 +474,8 @@ GetIterator(JSContext *cx, JSObject *obj, uintN flags, jsval *vp)
Compare(ni->shapes_array, shapes.begin(), ni->shapes_length)) {
*vp = OBJECT_TO_JSVAL(iterobj);
*hp = ni->next;
RegisterEnumerator(cx, iterobj, ni);
return true;
}
}
@ -486,6 +502,8 @@ GetIterator(JSContext *cx, JSObject *obj, uintN flags, jsval *vp)
return false;
iterobj->setNativeIterator(ni);
RegisterEnumerator(cx, iterobj, ni);
return true;
}
@ -627,8 +645,14 @@ js_CloseIterator(JSContext *cx, jsval v)
clasp = obj->getClass();
if (clasp == &js_IteratorClass.base) {
/* Cache the iterator object if possible. */
/* Remove enumerators from the active list, which is a stack. */
NativeIterator *ni = obj->getNativeIterator();
if (ni->flags & JSITER_ENUMERATE) {
JS_ASSERT(cx->enumerators == obj);
cx->enumerators = ni->next;
}
/* Cache the iterator object if possible. */
if (ni->shapes_length) {
uint32 hash = ni->shapes_key % NATIVE_ITER_CACHE_SIZE;
JSObject **hp = &JS_THREAD_DATA(cx)->cachedNativeIterators[hash];
@ -647,6 +671,67 @@ js_CloseIterator(JSContext *cx, jsval v)
return JS_TRUE;
}
/*
* Suppress enumeration of deleted properties. We maintain a list of all active
* non-escaping for-in enumerators. Whenever a property is deleted, we check
* whether any active enumerator contains the (obj, id) pair and has not
* enumerated id yet. If so, we delete the id from the list (or advance the
* cursor if it is the next id to be enumerated).
*
* We do not suppress enumeration of a property deleted along an object's
* prototype chain. Only direct deletions on the object are handled.
*/
bool
js_SuppressDeletedProperty(JSContext *cx, JSObject *obj, jsid id)
{
JSObject *iterobj = cx->enumerators;
while (iterobj) {
NativeIterator *ni = iterobj->getNativeIterator();
if (ni->obj == obj && ni->props_cursor < ni->props_end) {
/* Check whether id is still to come. */
for (jsid *idp = ni->props_cursor; idp < ni->props_end; ++idp) {
if (*idp == id) {
/*
* Check whether another property along the prototype chain
* became visible as a result of this deletion.
*/
if (obj->getProto()) {
AutoObjectRooter proto(cx, obj->getProto());
AutoObjectRooter obj2(cx);
JSProperty *prop;
if (!proto.object()->lookupProperty(cx, id, obj2.addr(), &prop))
return false;
if (obj2.object()) {
uintN attrs;
JSBool ok = obj2.object()->getAttributes(cx, id, prop, &attrs);
obj2.object()->dropProperty(cx, prop);
if (!ok)
return false;
if (attrs & JSPROP_ENUMERATE)
continue;
}
}
/*
* No property along the prototype chain steppeded in to take the
* property's place, so go ahead and delete id from the list.
* If it is the next property to be enumerated, just skip it.
*/
if (idp == ni->props_cursor) {
ni->props_cursor++;
} else {
memmove(idp, idp + 1, (ni->props_end - (idp + 1)) * sizeof(jsid));
ni->props_end--;
}
break;
}
}
}
iterobj = ni->next;
}
return true;
}
JSBool
js_IteratorMore(JSContext *cx, JSObject *iterobj, jsval *rval)
{
@ -848,6 +933,7 @@ js_NewGenerator(JSContext *cx)
JS_ASSERT(cx->regs->sp == fp->slots() + fp->script->nfixed);
gen->savedRegs.sp = slots + fp->script->nfixed;
gen->vplen = vplen;
gen->enumerators = NULL;
gen->liveFrame = newfp;
/* Copy generator's stack frame copy in from |cx->fp|. */
@ -995,8 +1081,16 @@ SendToGenerator(JSContext *cx, JSGeneratorOp op, JSObject *obj,
/* Officially push |fp|. |frame|'s destructor pops. */
cx->stack().pushExecuteFrame(cx, frame, gen->savedRegs, NULL);
/* Swap the enumerators stack for the generator's stack. */
JSObject *enumerators = cx->enumerators;
cx->enumerators = gen->enumerators;
ok = js_Interpret(cx);
/* Restore the original enumerators stack. */
gen->enumerators = cx->enumerators;
cx->enumerators = enumerators;
/* Restore call/args/block objects. */
cx->leaveGenerator(gen);
gen->liveFrame = genfp;

View File

@ -61,6 +61,7 @@ JS_BEGIN_EXTERN_C
#define JSITER_HIDDEN 0x10 /* also enumerate non-enumerable properties */
struct NativeIterator {
JSObject *obj;
jsval *props_array;
jsval *props_cursor;
jsval *props_end;
@ -70,7 +71,7 @@ struct NativeIterator {
uintN flags;
JSObject *next;
static NativeIterator *allocate(JSContext *cx, uintN flags,
static NativeIterator *allocate(JSContext *cx, JSObject *obj, uintN flags,
uint32 *sarray, uint32 slength, uint32 key,
jsval *parray, uint32 plength);
@ -91,7 +92,7 @@ bool
GetIterator(JSContext *cx, JSObject *obj, uintN flags, jsval *vp);
bool
JSIdArrayToIterator(JSContext *cx, uintN flags, JSIdArray *ida, jsval *vp);
JSIdArrayToIterator(JSContext *cx, JSObject *obj, uintN flags, JSIdArray *ida, jsval *vp);
/*
* Convert the value stored in *vp to its iteration object. The flags should
@ -105,6 +106,9 @@ js_ValueToIterator(JSContext *cx, uintN flags, jsval *vp);
extern JS_FRIEND_API(JSBool)
js_CloseIterator(JSContext *cx, jsval v);
bool
js_SuppressDeletedProperty(JSContext *cx, JSObject *obj, jsid id);
/*
* IteratorMore() indicates whether another value is available. It might
* internally call iterobj.next() and then cache the value until its
@ -138,6 +142,7 @@ struct JSGenerator {
JSFrameRegs savedRegs;
uintN vplen;
JSStackFrame *liveFrame;
JSObject *enumerators;
jsval floatingStack[1];
JSStackFrame *getFloatingFrame() {

View File

@ -64,6 +64,7 @@
#include "jsfun.h"
#include "jsgc.h"
#include "jsinterp.h"
#include "jsiter.h"
#include "jslock.h"
#include "jsnum.h"
#include "jsobj.h"
@ -5343,7 +5344,8 @@ js_DeleteProperty(JSContext *cx, JSObject *obj, jsid id, jsval *rval)
ok = scope->removeProperty(cx, id);
obj->dropProperty(cx, prop);
return ok;
return ok && js_SuppressDeletedProperty(cx, obj, id);
}
JSBool

View File

@ -195,7 +195,7 @@ JSProxyHandler::iterate(JSContext *cx, JSObject *proxy, uintN flags, jsval *vp)
if (!enumerate(cx, proxy, &ida))
return false;
AutoIdArray idar(cx, ida);
return JSIdArrayToIterator(cx, flags, ida, vp);
return JSIdArrayToIterator(cx, proxy, flags, ida, vp);
}
void

View File

@ -12,6 +12,6 @@ for each (var i in a)
checkStats({
recorderAborted:0,
traceCompleted:2,
sideExitIntoInterpreter:2
traceCompleted:1,
sideExitIntoInterpreter:1
});