Bug 633219 - replacing resolve hashtable with a linked list. r=luke

This commit is contained in:
Igor Bukanov 2011-03-07 23:00:00 +01:00
parent 7aa8609a45
commit 6990aee491
9 changed files with 299 additions and 263 deletions

View File

@ -0,0 +1,63 @@
// Test that the watch handler is not called recursively for the same object
// and property.
(function() {
var obj1 = {}, obj2 = {};
var handler_entry_count = 0;
var handler_exit_count = 0;
obj1.watch('x', handler);
obj1.watch('y', handler);
obj2.watch('x', handler);
obj1.x = 1;
assertEq(handler_entry_count, 3);
assertEq(handler_exit_count, 3);
function handler(id) {
handler_entry_count++;
assertEq(handler_exit_count, 0);
switch (true) {
case this === obj1 && id === "x":
assertEq(handler_entry_count, 1);
obj2.x = 3;
assertEq(handler_exit_count, 2);
break;
case this === obj2 && id === "x":
assertEq(handler_entry_count, 2);
obj1.y = 4;
assertEq(handler_exit_count, 1);
break;
default:
assertEq(this, obj1);
assertEq(id, "y");
assertEq(handler_entry_count, 3);
// We expect no more watch handler invocations
obj1.x = 5;
obj1.y = 6;
obj2.x = 7;
assertEq(handler_exit_count, 0);
break;
}
++handler_exit_count;
assertEq(handler_entry_count, 3);
}
})();
// Test that run-away recursion in watch handlers is properly handled.
(function() {
var obj = {};
var i = 0;
try {
handler();
throw new Error("Unreachable");
} catch(e) {
assertEq(e instanceof InternalError, true);
}
function handler() {
var prop = "a" + ++i;
obj.watch(prop, handler);
obj[prop] = 2;
}
})();

View File

@ -67,6 +67,7 @@ CPPSRCS = \
testNewObject.cpp \
testOps.cpp \
testPropCache.cpp \
testResolveRecursion.cpp \
testSameValue.cpp \
testScriptObject.cpp \
testSetProperty.cpp \

View File

@ -0,0 +1,134 @@
/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
* vim: set ts=8 sw=4 et tw=99:
*/
#include "tests.h"
/*
* Test that resolve hook recursion for the same object and property is
* prevented.
*/
BEGIN_TEST(testResolveRecursion)
{
static JSClass my_resolve_class = {
"MyResolve",
JSCLASS_NEW_RESOLVE | JSCLASS_HAS_PRIVATE,
JS_PropertyStub, // add
JS_PropertyStub, // delete
JS_PropertyStub, // get
JS_StrictPropertyStub, // set
JS_EnumerateStub,
(JSResolveOp) my_resolve,
JS_ConvertStub,
JS_FinalizeStub,
JSCLASS_NO_OPTIONAL_MEMBERS
};
obj1 = JS_NewObject(cx, &my_resolve_class, NULL, NULL);
CHECK(obj1);
obj2 = JS_NewObject(cx, &my_resolve_class, NULL, NULL);
CHECK(obj2);
CHECK(JS_SetPrivate(cx, obj1, this));
CHECK(JS_SetPrivate(cx, obj2, this));
CHECK(JS_DefineProperty(cx, global, "obj1", OBJECT_TO_JSVAL(obj1), NULL, NULL, 0));
CHECK(JS_DefineProperty(cx, global, "obj2", OBJECT_TO_JSVAL(obj2), NULL, NULL, 0));
resolveEntryCount = 0;
resolveExitCount = 0;
/* Start the essence of the test via invoking the first resolve hook. */
jsval v;
EVAL("obj1.x", &v);
CHECK(v == JSVAL_FALSE);
CHECK(resolveEntryCount == 4);
CHECK(resolveExitCount == 4);
return true;
}
JSObject *obj1;
JSObject *obj2;
unsigned resolveEntryCount;
unsigned resolveExitCount;
struct AutoIncrCounters {
AutoIncrCounters(cls_testResolveRecursion *t) : t(t) {
t->resolveEntryCount++;
}
~AutoIncrCounters() {
t->resolveExitCount++;
}
cls_testResolveRecursion *t;
};
bool
doResolve(JSObject *obj, jsid id, uintN flags, JSObject **objp)
{
CHECK(resolveExitCount == 0);
AutoIncrCounters incr(this);
CHECK(obj == obj1 || obj == obj2);
CHECK(JSID_IS_STRING(id));
JSFlatString *str = JS_FlattenString(cx, JSID_TO_STRING(id));
CHECK(str);
jsval v;
if (JS_FlatStringEqualsAscii(str, "x")) {
if (obj == obj1) {
/* First resolve hook invocation. */
CHECK(resolveEntryCount == 1);
EVAL("obj2.y = true", &v);
CHECK(v == JSVAL_TRUE);
CHECK(JS_DefinePropertyById(cx, obj, id, JSVAL_FALSE, NULL, NULL, 0));
*objp = obj;
return true;
}
if (obj == obj2) {
CHECK(resolveEntryCount == 4);
*objp = NULL;
return true;
}
} else if (JS_FlatStringEqualsAscii(str, "y")) {
if (obj == obj2) {
CHECK(resolveEntryCount == 2);
CHECK(JS_DefinePropertyById(cx, obj, id, JSVAL_NULL, NULL, NULL, 0));
EVAL("obj1.x", &v);
CHECK(JSVAL_IS_VOID(v));
EVAL("obj1.y", &v);
CHECK(v == JSVAL_ZERO);
*objp = obj;
return true;
}
if (obj == obj1) {
CHECK(resolveEntryCount == 3);
EVAL("obj1.x", &v);
CHECK(JSVAL_IS_VOID(v));
EVAL("obj1.y", &v);
CHECK(JSVAL_IS_VOID(v));
EVAL("obj2.y", &v);
CHECK(JSVAL_IS_NULL(v));
EVAL("obj2.x", &v);
CHECK(JSVAL_IS_VOID(v));
EVAL("obj1.y = 0", &v);
CHECK(v == JSVAL_ZERO);
*objp = obj;
return true;
}
}
CHECK(false);
return false;
}
static JSBool
my_resolve(JSContext *cx, JSObject *obj, jsid id, uintN flags, JSObject **objp)
{
return static_cast<cls_testResolveRecursion *>(JS_GetPrivate(cx, obj))->
doResolve(obj, id, flags, objp);
}
END_TEST(testResolveRecursion)

View File

@ -1498,37 +1498,6 @@ JS_SetGlobalObject(JSContext *cx, JSObject *obj)
cx->resetCompartment();
}
class AutoResolvingEntry {
public:
AutoResolvingEntry() : entry(NULL) {}
/*
* Returns false on error. But N.B. if obj[id] was already being resolved,
* this is a no-op, and we silently treat that as success.
*/
bool start(JSContext *cx, JSObject *obj, jsid id, uint32 flag) {
JS_ASSERT(!entry);
this->cx = cx;
key.obj = obj;
key.id = id;
this->flag = flag;
bool ok = !!js_StartResolving(cx, &key, flag, &entry);
JS_ASSERT_IF(!ok, !entry);
return ok;
}
~AutoResolvingEntry() {
if (entry)
js_StopResolving(cx, &key, flag, NULL, 0);
}
private:
JSContext *cx;
JSResolvingKey key;
uint32 flag;
JSResolvingEntry *entry;
};
JSObject *
js_InitFunctionAndObjectClasses(JSContext *cx, JSObject *obj)
{
@ -1539,13 +1508,10 @@ js_InitFunctionAndObjectClasses(JSContext *cx, JSObject *obj)
if (!cx->globalObject)
JS_SetGlobalObject(cx, obj);
/* Record Function and Object in cx->resolvingTable. */
AutoResolvingEntry e1, e2;
/* Record Function and Object in cx->resolvingList. */
JSAtom **classAtoms = cx->runtime->atomState.classAtoms;
if (!e1.start(cx, obj, ATOM_TO_JSID(classAtoms[JSProto_Function]), JSRESFLAG_LOOKUP) ||
!e2.start(cx, obj, ATOM_TO_JSID(classAtoms[JSProto_Object]), JSRESFLAG_LOOKUP)) {
return NULL;
}
AutoResolving resolving1(cx, obj, ATOM_TO_JSID(classAtoms[JSProto_Function]));
AutoResolving resolving2(cx, obj, ATOM_TO_JSID(classAtoms[JSProto_Object]));
/* Initialize the function class first so constructors can be made. */
if (!js_GetClassPrototype(cx, obj, JSProto_Function, &fun_proto))

View File

@ -1131,11 +1131,7 @@ FreeContext(JSContext *cx)
cx->free(temp);
}
/* Destroy the resolve recursion damper. */
if (cx->resolvingTable) {
JS_DHashTableDestroy(cx->resolvingTable);
cx->resolvingTable = NULL;
}
JS_ASSERT(!cx->resolvingList);
/* Finally, free cx itself. */
cx->~JSContext();
@ -1170,107 +1166,22 @@ js_NextActiveContext(JSRuntime *rt, JSContext *cx)
#endif
}
static JSDHashNumber
resolving_HashKey(JSDHashTable *table, const void *ptr)
{
const JSResolvingKey *key = (const JSResolvingKey *)ptr;
namespace js {
return (JSDHashNumber(uintptr_t(key->obj)) >> JS_GCTHING_ALIGN) ^ JSID_BITS(key->id);
bool
AutoResolving::alreadyStartedSlow() const
{
JS_ASSERT(link);
AutoResolving *cursor = link;
do {
JS_ASSERT(this != cursor);
if (object == cursor->object && id == cursor->id && kind == cursor->kind)
return true;
} while (!!(cursor = cursor->link));
return false;
}
static JSBool
resolving_MatchEntry(JSDHashTable *table,
const JSDHashEntryHdr *hdr,
const void *ptr)
{
const JSResolvingEntry *entry = (const JSResolvingEntry *)hdr;
const JSResolvingKey *key = (const JSResolvingKey *)ptr;
return entry->key.obj == key->obj && entry->key.id == key->id;
}
static const JSDHashTableOps resolving_dhash_ops = {
JS_DHashAllocTable,
JS_DHashFreeTable,
resolving_HashKey,
resolving_MatchEntry,
JS_DHashMoveEntryStub,
JS_DHashClearEntryStub,
JS_DHashFinalizeStub,
NULL
};
JSBool
js_StartResolving(JSContext *cx, JSResolvingKey *key, uint32 flag,
JSResolvingEntry **entryp)
{
JSDHashTable *table;
JSResolvingEntry *entry;
table = cx->resolvingTable;
if (!table) {
table = JS_NewDHashTable(&resolving_dhash_ops, NULL,
sizeof(JSResolvingEntry),
JS_DHASH_MIN_SIZE);
if (!table)
goto outofmem;
cx->resolvingTable = table;
}
entry = (JSResolvingEntry *)
JS_DHashTableOperate(table, key, JS_DHASH_ADD);
if (!entry)
goto outofmem;
if (entry->flags & flag) {
/* An entry for (key, flag) exists already -- dampen recursion. */
entry = NULL;
} else {
/* Fill in key if we were the first to add entry, then set flag. */
if (!entry->key.obj)
entry->key = *key;
entry->flags |= flag;
}
*entryp = entry;
return JS_TRUE;
outofmem:
JS_ReportOutOfMemory(cx);
return JS_FALSE;
}
void
js_StopResolving(JSContext *cx, JSResolvingKey *key, uint32 flag,
JSResolvingEntry *entry, uint32 generation)
{
JSDHashTable *table;
/*
* Clear flag from entry->flags and return early if other flags remain.
* We must take care to re-lookup entry if the table has changed since
* it was found by js_StartResolving.
*/
table = cx->resolvingTable;
if (!entry || table->generation != generation) {
entry = (JSResolvingEntry *)
JS_DHashTableOperate(table, key, JS_DHASH_LOOKUP);
}
JS_ASSERT(JS_DHASH_ENTRY_IS_BUSY(&entry->hdr));
entry->flags &= ~flag;
if (entry->flags)
return;
/*
* Do a raw remove only if fewer entries were removed than would cause
* alpha to be less than .5 (alpha is at most .75). Otherwise, we just
* call JS_DHashTableOperate to re-lookup the key and remove its entry,
* compressing or shrinking the table as needed.
*/
if (table->removedCount < JS_DHASH_TABLE_SIZE(table) >> 2)
JS_DHashTableRawRemove(table, &entry->hdr);
else
JS_DHashTableOperate(table, key, JS_DHASH_REMOVE);
}
} /* namespace js */
static void
ReportError(JSContext *cx, const char *message, JSErrorReport *reportp,

View File

@ -1466,32 +1466,12 @@ struct JSArgumentFormatMap {
};
#endif
/*
* Key and entry types for the JSContext.resolvingTable hash table, typedef'd
* here because all consumers need to see these declarations (and not just the
* typedef names, as would be the case for an opaque pointer-to-typedef'd-type
* declaration), along with cx->resolvingTable.
*/
typedef struct JSResolvingKey {
JSObject *obj;
jsid id;
} JSResolvingKey;
typedef struct JSResolvingEntry {
JSDHashEntryHdr hdr;
JSResolvingKey key;
uint32 flags;
} JSResolvingEntry;
#define JSRESFLAG_LOOKUP 0x1 /* resolving id from lookup */
#define JSRESFLAG_WATCH 0x2 /* resolving id from watch */
#define JSRESOLVE_INFER 0xffff /* infer bits from current bytecode */
extern const JSDebugHooks js_NullDebugHooks; /* defined in jsdbgapi.cpp */
namespace js {
class AutoGCRooter;
struct AutoResolving;
static inline bool
OptionsHasXML(uint32 options)
@ -1641,13 +1621,7 @@ struct JSContext
/* Locale specific callbacks for string conversion. */
JSLocaleCallbacks *localeCallbacks;
/*
* cx->resolvingTable is non-null and non-empty if we are initializing
* standard classes lazily, or if we are otherwise recursing indirectly
* from js_LookupProperty through a Class.resolve hook. It is used to
* limit runaway recursion (see jsapi.c and jsobj.c).
*/
JSDHashTable *resolvingTable;
js::AutoResolving *resolvingList;
/*
* True if generating an error, to prevent runaway recursion.
@ -2181,6 +2155,42 @@ FrameAtomBase(JSContext *cx, JSStackFrame *fp)
namespace js {
struct AutoResolving {
public:
enum Kind {
LOOKUP,
WATCH
};
AutoResolving(JSContext *cx, JSObject *obj, jsid id, Kind kind = LOOKUP
JS_GUARD_OBJECT_NOTIFIER_PARAM)
: context(cx), object(obj), id(id), kind(kind), link(cx->resolvingList)
{
JS_GUARD_OBJECT_NOTIFIER_INIT;
JS_ASSERT(obj);
cx->resolvingList = this;
}
~AutoResolving() {
JS_ASSERT(context->resolvingList == this);
context->resolvingList = link;
}
bool alreadyStarted() const {
return link && alreadyStartedSlow();
}
private:
bool alreadyStartedSlow() const;
JSContext *const context;
JSObject *const object;
jsid const id;
Kind const kind;
AutoResolving *const link;
JS_DECL_USE_GUARD_OBJECT_NOTIFIER
};
class AutoGCRooter {
public:
AutoGCRooter(JSContext *cx, ptrdiff_t tag)
@ -2977,17 +2987,6 @@ js_ContextIterator(JSRuntime *rt, JSBool unlocked, JSContext **iterp);
extern JS_FRIEND_API(JSContext *)
js_NextActiveContext(JSRuntime *, JSContext *);
/*
* Class.resolve and watchpoint recursion damping machinery.
*/
extern JSBool
js_StartResolving(JSContext *cx, JSResolvingKey *key, uint32 flag,
JSResolvingEntry **entryp);
extern void
js_StopResolving(JSContext *cx, JSResolvingKey *key, uint32 flag,
JSResolvingEntry *entry, uint32 generation);
/*
* Report an exception, which is currently realized as a printf-style format
* string and its arguments.

View File

@ -1328,53 +1328,33 @@ static JSBool
obj_watch_handler(JSContext *cx, JSObject *obj, jsid id, jsval old,
jsval *nvp, void *closure)
{
JSObject *callable;
JSSecurityCallbacks *callbacks;
JSStackFrame *caller;
JSPrincipals *subject, *watcher;
JSResolvingKey key;
JSResolvingEntry *entry;
uint32 generation;
Value argv[3];
JSBool ok;
callable = (JSObject *) closure;
callbacks = JS_GetSecurityCallbacks(cx);
JSObject *callable = (JSObject *) closure;
JSSecurityCallbacks *callbacks = JS_GetSecurityCallbacks(cx);
if (callbacks && callbacks->findObjectPrincipals) {
/* Skip over any obj_watch_* frames between us and the real subject. */
caller = js_GetScriptedCaller(cx, NULL);
if (caller) {
if (JSStackFrame *caller = js_GetScriptedCaller(cx, NULL)) {
/*
* Only call the watch handler if the watcher is allowed to watch
* the currently executing script.
*/
watcher = callbacks->findObjectPrincipals(cx, callable);
subject = js_StackFramePrincipals(cx, caller);
JSPrincipals *watcher = callbacks->findObjectPrincipals(cx, callable);
JSPrincipals *subject = js_StackFramePrincipals(cx, caller);
if (watcher && subject && !watcher->subsume(watcher, subject)) {
/* Silently don't call the watch handler. */
return JS_TRUE;
return true;
}
}
}
/* Avoid recursion on (obj, id) already being watched on cx. */
key.obj = obj;
key.id = id;
if (!js_StartResolving(cx, &key, JSRESFLAG_WATCH, &entry))
return JS_FALSE;
if (!entry)
return JS_TRUE;
generation = cx->resolvingTable->generation;
AutoResolving resolving(cx, obj, id, AutoResolving::WATCH);
if (resolving.alreadyStarted())
return true;
argv[0] = IdToValue(id);
argv[1] = Valueify(old);
argv[2] = Valueify(*nvp);
ok = ExternalInvoke(cx, ObjectValue(*obj), ObjectOrNullValue(callable), 3, argv,
Valueify(nvp));
js_StopResolving(cx, &key, JSRESFLAG_WATCH, entry, generation);
return ok;
Value argv[] = { IdToValue(id), Valueify(old), Valueify(*nvp) };
return ExternalInvoke(cx, ObjectValue(*obj), ObjectOrNullValue(callable),
JS_ARRAY_LENGTH(argv), argv, Valueify(nvp));
}
static JSBool
@ -4174,52 +4154,36 @@ JSBool
js_GetClassObject(JSContext *cx, JSObject *obj, JSProtoKey key,
JSObject **objp)
{
JSObject *cobj;
JSResolvingKey rkey;
JSResolvingEntry *rentry;
uint32 generation;
JSObjectOp init;
Value v;
obj = obj->getGlobal();
if (!obj->isGlobal()) {
*objp = NULL;
return JS_TRUE;
return true;
}
v = obj->getReservedSlot(key);
Value v = obj->getReservedSlot(key);
if (v.isObject()) {
*objp = &v.toObject();
return JS_TRUE;
return true;
}
rkey.obj = obj;
rkey.id = ATOM_TO_JSID(cx->runtime->atomState.classAtoms[key]);
if (!js_StartResolving(cx, &rkey, JSRESFLAG_LOOKUP, &rentry))
return JS_FALSE;
if (!rentry) {
/* Already caching key in obj -- suppress recursion. */
AutoResolving resolving(cx, obj, ATOM_TO_JSID(cx->runtime->atomState.classAtoms[key]));
if (resolving.alreadyStarted()) {
/* Already caching id in obj -- suppress recursion. */
*objp = NULL;
return JS_TRUE;
}
generation = cx->resolvingTable->generation;
JSBool ok = true;
cobj = NULL;
init = lazy_prototype_init[key];
if (init) {
if (!init(cx, obj)) {
ok = JS_FALSE;
} else {
v = obj->getReservedSlot(key);
if (v.isObject())
cobj = &v.toObject();
}
return true;
}
JSObject *cobj = NULL;
if (JSObjectOp init = lazy_prototype_init[key]) {
if (!init(cx, obj))
return false;
v = obj->getReservedSlot(key);
if (v.isObject())
cobj = &v.toObject();
}
js_StopResolving(cx, &rkey, JSRESFLAG_LOOKUP, rentry, generation);
*objp = cobj;
return ok;
return true;
}
JSBool
@ -4863,29 +4827,23 @@ CallResolveOp(JSContext *cx, JSObject *start, JSObject *obj, jsid id, uintN flag
* returning. But note that JS_DHASH_ADD may find an existing
* entry, in which case we bail to suppress runaway recursion.
*/
JSResolvingKey key = {obj, id};
JSResolvingEntry *entry;
if (!js_StartResolving(cx, &key, JSRESFLAG_LOOKUP, &entry))
return false;
if (!entry) {
AutoResolving resolving(cx, obj, id);
if (resolving.alreadyStarted()) {
/* Already resolving id in obj -- suppress recursion. */
*recursedp = true;
return true;
}
uint32 generation = cx->resolvingTable->generation;
*recursedp = false;
*propp = NULL;
JSBool ok;
if (clasp->flags & JSCLASS_NEW_RESOLVE) {
JSNewResolveOp newresolve = reinterpret_cast<JSNewResolveOp>(resolve);
if (flags == JSRESOLVE_INFER)
flags = js_InferFlags(cx, 0);
JSObject *obj2 = (clasp->flags & JSCLASS_NEW_RESOLVE_GETS_START) ? start : NULL;
ok = newresolve(cx, obj, id, flags, &obj2);
if (!ok)
goto cleanup;
if (!newresolve(cx, obj, id, flags, &obj2))
return false;
/*
* We trust the new style resolve hook to set obj2 to NULL when
@ -4894,19 +4852,17 @@ CallResolveOp(JSContext *cx, JSObject *start, JSObject *obj, jsid id, uintN flag
* compatibility.
*/
if (!obj2)
goto cleanup;
return true;
if (!obj2->isNative()) {
/* Whoops, newresolve handed back a foreign obj2. */
JS_ASSERT(obj2 != obj);
ok = obj2->lookupProperty(cx, id, objp, propp);
goto cleanup;
return obj2->lookupProperty(cx, id, objp, propp);
}
obj = obj2;
} else {
ok = resolve(cx, obj, id);
if (!ok)
goto cleanup;
if (!resolve(cx, obj, id))
return false;
}
if (!obj->nativeEmpty()) {
@ -4916,9 +4872,7 @@ CallResolveOp(JSContext *cx, JSObject *start, JSObject *obj, jsid id, uintN flag
}
}
cleanup:
js_StopResolving(cx, &key, JSRESFLAG_LOOKUP, entry, generation);
return ok;
return true;
}
static JS_ALWAYS_INLINE int

View File

@ -1675,6 +1675,11 @@ extern int
js_LookupPropertyWithFlags(JSContext *cx, JSObject *obj, jsid id, uintN flags,
JSObject **objp, JSProperty **propp);
/*
* Constant to pass to js_LookupPropertyWithFlags to infer bits from current
* bytecode.
*/
static const uintN JSRESOLVE_INFER = 0xffff;
extern JS_FRIEND_DATA(js::Class) js_CallClass;
extern JS_FRIEND_DATA(js::Class) js_DeclEnvClass;

View File

@ -399,6 +399,8 @@ public:
JSGuardObjectNotificationReceiver _mCheckNotUsedAsTemporary;
#define JS_GUARD_OBJECT_NOTIFIER_PARAM \
, const JSGuardObjectNotifier& _notifier = JSGuardObjectNotifier()
#define JS_GUARD_OBJECT_NOTIFIER_PARAM_NO_INIT \
, const JSGuardObjectNotifier& _notifier
#define JS_GUARD_OBJECT_NOTIFIER_PARAM0 \
const JSGuardObjectNotifier& _notifier = JSGuardObjectNotifier()
#define JS_GUARD_OBJECT_NOTIFIER_INIT \
@ -408,6 +410,7 @@ public:
#define JS_DECL_USE_GUARD_OBJECT_NOTIFIER
#define JS_GUARD_OBJECT_NOTIFIER_PARAM
#define JS_GUARD_OBJECT_NOTIFIER_PARAM_NO_INIT
#define JS_GUARD_OBJECT_NOTIFIER_PARAM0
#define JS_GUARD_OBJECT_NOTIFIER_INIT JS_BEGIN_MACRO JS_END_MACRO