From 6ccb8a044ce07c0f7d2dbdb6c16ff5925bf33392 Mon Sep 17 00:00:00 2001 From: "brendan%mozilla.org" Date: Tue, 29 Apr 2008 06:19:42 +0000 Subject: [PATCH] Fix hang when GetPropertyTreeChild calls js_GenerateShape calls js_GC (424636, r=igor, a=beltzner). git-svn-id: svn://10.0.0.236/trunk@250910 18797224-902f-48f8-a5cc-f745e15eee43 --- mozilla/js/src/jsinterp.c | 4 ++-- mozilla/js/src/jsinterp.h | 2 +- mozilla/js/src/jsscope.c | 41 +++++++++++++++++++++------------------ mozilla/js/src/jsscope.h | 4 ++-- mozilla/js/src/jsstr.h | 24 +++++++++++------------ 5 files changed, 38 insertions(+), 37 deletions(-) diff --git a/mozilla/js/src/jsinterp.c b/mozilla/js/src/jsinterp.c index ecbd5bc3c6f..4aa8fabac4a 100644 --- a/mozilla/js/src/jsinterp.c +++ b/mozilla/js/src/jsinterp.c @@ -80,7 +80,7 @@ #ifdef js_invoke_c__ uint32 -js_GenerateShape(JSContext *cx) +js_GenerateShape(JSContext *cx, JSBool gcLocked) { JSRuntime *rt; uint32 shape; @@ -90,7 +90,7 @@ js_GenerateShape(JSContext *cx) JS_ASSERT(shape != 0); if (shape & SHAPE_OVERFLOW_BIT) { rt->gcPoke = JS_TRUE; - js_GC(cx, GC_NORMAL); + js_GC(cx, gcLocked ? GC_LOCK_HELD : GC_NORMAL); shape = JS_ATOMIC_INCREMENT(&rt->shapeGen); JS_ASSERT(shape != 0); JS_ASSERT_IF(shape & SHAPE_OVERFLOW_BIT, diff --git a/mozilla/js/src/jsinterp.h b/mozilla/js/src/jsinterp.h index 419a744dc2c..f6db66426be 100644 --- a/mozilla/js/src/jsinterp.h +++ b/mozilla/js/src/jsinterp.h @@ -163,7 +163,7 @@ typedef struct JSInlineFrame { #define SHAPE_OVERFLOW_BIT JS_BIT(32 - PCVCAP_TAGBITS) extern uint32 -js_GenerateShape(JSContext *cx); +js_GenerateShape(JSContext *cx, JSBool gcLocked); struct JSPropCacheEntry { jsbytecode *kpc; /* pc if vcap tag is <= 1, else atom */ diff --git a/mozilla/js/src/jsscope.c b/mozilla/js/src/jsscope.c index 450c24dfd91..db5bab41ab0 100644 --- a/mozilla/js/src/jsscope.c +++ b/mozilla/js/src/jsscope.c @@ -490,7 +490,7 @@ typedef struct FreeNode { FREENODE(FREENODE(sprop)->next)->prevp = FREENODE(sprop)->prevp; \ JS_END_MACRO -/* NB: Called with the runtime lock held. */ +/* NB: Called with rt->gcLock held. */ static JSScopeProperty * NewScopeProperty(JSRuntime *rt) { @@ -551,7 +551,7 @@ DestroyPropTreeKidsChunk(JSRuntime *rt, PropTreeKidsChunk *chunk) free(chunk); } -/* NB: Called with the runtime lock held. */ +/* NB: Called with rt->gcLock held. */ static JSBool InsertPropertyTreeChild(JSRuntime *rt, JSScopeProperty *parent, JSScopeProperty *child, PropTreeKidsChunk *sweptChunk) @@ -686,7 +686,7 @@ InsertPropertyTreeChild(JSRuntime *rt, JSScopeProperty *parent, return JS_TRUE; } -/* NB: Called with the runtime lock held. */ +/* NB: Called with rt->gcLock held. */ static PropTreeKidsChunk * RemovePropertyTreeChild(JSRuntime *rt, JSScopeProperty *child) { @@ -792,9 +792,12 @@ HashChunks(PropTreeKidsChunk *chunk, uintN n) } /* - * Called *without* the runtime lock held, this function acquires that lock - * only when inserting a new child. Thus there may be races to find or add - * a node that result in duplicates. We expect such races to be rare! + * Called without cx->runtime->gcLock held. This function acquires that lock + * only when inserting a new child. Thus there may be races to find or add a + * node that result in duplicates. We expect such races to be rare! + * + * We use rt->gcLock, not rt->rtLock, to allow the GC potentially to nest here + * under js_GenerateShape. */ static JSScopeProperty * GetPropertyTreeChild(JSContext *cx, JSScopeProperty *parent, @@ -809,7 +812,7 @@ GetPropertyTreeChild(JSContext *cx, JSScopeProperty *parent, rt = cx->runtime; if (!parent) { - JS_LOCK_RUNTIME(rt); + JS_LOCK_GC(rt); table = &rt->propertyTreeHash; entry = (JSPropertyTreeEntry *) @@ -823,11 +826,11 @@ GetPropertyTreeChild(JSContext *cx, JSScopeProperty *parent, } else { /* * Because chunks are appended at the end and never deleted except by - * the GC, we can search without taking the runtime lock. We may miss - * a matching sprop added by another thread, and make a duplicate one, - * but that is an unlikely, therefore small, cost. The property tree - * has extremely low fan-out below its root in popular embeddings with - * real-world workloads. + * the GC, we can search without taking the runtime's GC lock. We may + * miss a matching sprop added by another thread, and make a duplicate + * one, but that is an unlikely, therefore small, cost. The property + * tree has extremely low fan-out below its root in popular embeddings + * with real-world workloads. * * Patterns such as defining closures that capture a constructor's * environment as getters or setters on the new object that is passed @@ -842,12 +845,12 @@ GetPropertyTreeChild(JSContext *cx, JSScopeProperty *parent, table = chunk->table; if (table) { - JS_LOCK_RUNTIME(rt); + JS_LOCK_GC(rt); entry = (JSPropertyTreeEntry *) JS_DHashTableOperate(table, child, JS_DHASH_LOOKUP); sprop = entry->child; if (sprop) { - JS_UNLOCK_RUNTIME(rt); + JS_UNLOCK_GC(rt); return sprop; } goto locked_not_found; @@ -863,7 +866,7 @@ GetPropertyTreeChild(JSContext *cx, JSScopeProperty *parent, chunk = KIDS_TO_CHUNK(parent->kids); if (!chunk->table) { table = HashChunks(chunk, n); - JS_LOCK_RUNTIME(rt); + JS_LOCK_GC(rt); if (!table) goto out_of_memory; if (chunk->table) @@ -888,7 +891,7 @@ GetPropertyTreeChild(JSContext *cx, JSScopeProperty *parent, } not_found: - JS_LOCK_RUNTIME(rt); + JS_LOCK_GC(rt); } locked_not_found: @@ -904,7 +907,7 @@ locked_not_found: sprop->flags = child->flags; sprop->shortid = child->shortid; sprop->parent = sprop->kids = NULL; - sprop->shape = js_GenerateShape(cx); + sprop->shape = js_GenerateShape(cx, JS_TRUE); if (!parent) { entry->child = sprop; @@ -914,11 +917,11 @@ locked_not_found: } out: - JS_UNLOCK_RUNTIME(rt); + JS_UNLOCK_GC(rt); return sprop; out_of_memory: - JS_UNLOCK_RUNTIME(rt); + JS_UNLOCK_GC(rt); JS_ReportOutOfMemory(cx); return NULL; } diff --git a/mozilla/js/src/jsscope.h b/mozilla/js/src/jsscope.h index 748e269a708..7f181dd2f22 100644 --- a/mozilla/js/src/jsscope.h +++ b/mozilla/js/src/jsscope.h @@ -220,7 +220,7 @@ JS_STATIC_ASSERT(offsetof(JSScope, title) == sizeof(JSObjectMap)); #define OBJ_SCOPE(obj) ((JSScope *)(obj)->map) #define SCOPE_MAKE_UNIQUE_SHAPE(cx,scope) \ - ((scope)->shape = js_GenerateShape(cx)) + ((scope)->shape = js_GenerateShape((cx), JS_FALSE)) #define SCOPE_EXTEND_SHAPE(cx,scope,sprop) \ JS_BEGIN_MACRO \ @@ -228,7 +228,7 @@ JS_STATIC_ASSERT(offsetof(JSScope, title) == sizeof(JSObjectMap)); (scope)->shape == (scope)->lastProp->shape) { \ (scope)->shape = (sprop)->shape; \ } else { \ - (scope)->shape = js_GenerateShape(cx); \ + (scope)->shape = js_GenerateShape((cx), JS_FALSE); \ } \ JS_END_MACRO diff --git a/mozilla/js/src/jsstr.h b/mozilla/js/src/jsstr.h index 1add234d3da..3432daf48ea 100644 --- a/mozilla/js/src/jsstr.h +++ b/mozilla/js/src/jsstr.h @@ -64,20 +64,18 @@ JS_BEGIN_EXTERN_C * purely a backstop, in case the chars pointer flows out to native code that * requires \u0000 termination. * - * A flat string with JSSTRFLAG_MUTABLE set means the string is accessible + * A flat string with JSSTRFLAG_MUTABLE set means that the string is accessible * only from one thread and it is possible to turn it into a dependent string - * of the same length to optimize js_ConcatStrings. It also possible to grow - * such string but extreme care must be taken to ensure that no other code + * of the same length to optimize js_ConcatStrings. It is also possible to grow + * such a string, but extreme care must be taken to ensure that no other code * relies on the original length of the string. * - * A flat string with JSSTRFLAG_ATOMIZED set means that the string is hashed - * as an atom. This flag is used to avoid re-hashing of the already-atomized - * string. + * A flat string with JSSTRFLAG_ATOMIZED set means that the string is hashed as + * an atom. This flag is used to avoid re-hashing the already-atomized string. * - * When JSSTRFLAG_DEPENDENT is set, the string depends on characters of - * another string strongly referenced by the u.base field. The base member may - * point to another dependent string if JSSTRING_CHARS has not been called - * yet. + * When JSSTRFLAG_DEPENDENT is set, the string depends on characters of another + * string strongly referenced by the u.base field. The base member may point to + * another dependent string if JSSTRING_CHARS has not been called yet. * * JSSTRFLAG_PREFIX determines the kind of the dependent string. When the flag * is unset, the length field encodes both starting position relative to the @@ -85,9 +83,9 @@ JS_BEGIN_EXTERN_C * JSSTRDEP_START_MASK and JSSTRDEP_LENGTH_MASK macros below for details. * * When JSSTRFLAG_PREFIX is set, the dependent string is a prefix of the base - * string. The number of characters in the prefix is encoded using all - * non-flag bits of the length field and spans the same 0 .. SIZE_T_MAX/4 - * range as the length of the flat string. + * string. The number of characters in the prefix is encoded using all non-flag + * bits of the length field and spans the same 0 .. SIZE_T_MAX/4 range as the + * length of the flat string. * * NB: Always use the JSSTRING_LENGTH and JSSTRING_CHARS accessor macros. */