diff --git a/mozilla/js/src/jsapi.c b/mozilla/js/src/jsapi.c index ffc80b4a18f..40c4cd67a72 100644 --- a/mozilla/js/src/jsapi.c +++ b/mozilla/js/src/jsapi.c @@ -5794,21 +5794,62 @@ JS_PUBLIC_API(jsword) JS_SetContextThread(JSContext *cx) { #ifdef JS_THREADSAFE - jsword old = JS_THREAD_ID(cx); - if (!js_SetContextThread(cx)) + JSRuntime *rt; + JSThread *thread; + + JS_ASSERT(cx->requestDepth == 0); + if (cx->thread) { + JS_ASSERT(cx->thread->id == js_CurrentThreadId()); + return cx->thread->id; + } + + rt = cx->runtime; + thread = js_GetCurrentThread(rt); + if (!thread) { + js_ReportOutOfMemory(cx); return -1; - return old; -#else - return 0; + } + + /* + * We must not race with a GC that accesses cx->thread for all threads, + * see bug 476934. + */ + JS_LOCK_GC(rt); + js_WaitForGC(rt); + js_InitContextThread(cx, thread); + JS_UNLOCK_GC(rt); #endif + return 0; } JS_PUBLIC_API(jsword) JS_ClearContextThread(JSContext *cx) { #ifdef JS_THREADSAFE - jsword old = JS_THREAD_ID(cx); - js_ClearContextThread(cx); + jsword old; + JSRuntime *rt; + + /* + * This must be called outside a request and, if cx is associated with a + * thread, this must be called only from that thread. If not, this is a + * harmless no-op. + */ + JS_ASSERT(cx->requestDepth == 0); + if (!cx->thread) + return 0; + old = cx->thread->id; + JS_ASSERT(old == js_CurrentThreadId()); + + /* + * We must not race with a GC that accesses cx->thread for all threads, + * see bug 476934. + */ + rt = cx->runtime; + JS_LOCK_GC(rt); + js_WaitForGC(rt); + JS_REMOVE_AND_INIT_LINK(&cx->threadLinks); + cx->thread = NULL; + JS_UNLOCK_GC(cx->runtime); return old; #else return 0; diff --git a/mozilla/js/src/jscntxt.c b/mozilla/js/src/jscntxt.c index 69e7995f6d8..942479bb038 100644 --- a/mozilla/js/src/jscntxt.c +++ b/mozilla/js/src/jscntxt.c @@ -146,7 +146,7 @@ js_GetCurrentThread(JSRuntime *rt) thread->gcMallocBytes = 0; /* - * js_SetContextThread initializes the remaining fields as necessary. + * js_InitContextThread initializes the remaining fields as necessary. */ } return thread; @@ -154,18 +154,14 @@ js_GetCurrentThread(JSRuntime *rt) /* * Sets current thread as owning thread of a context by assigning the - * thread-private info to the context. If the current thread doesn't have - * private JSThread info, create one. + * thread-private info to the context. */ -JSBool -js_SetContextThread(JSContext *cx) +void +js_InitContextThread(JSContext *cx, JSThread *thread) { - JSThread *thread = js_GetCurrentThread(cx->runtime); - - if (!thread) { - JS_ReportOutOfMemory(cx); - return JS_FALSE; - } + JS_ASSERT(CURRENT_THREAD_IS_ME(thread)); + JS_ASSERT(!cx->thread); + JS_ASSERT(cx->requestDepth == 0); /* * Clear gcFreeLists and caches on each transition from 0 to 1 context @@ -177,31 +173,8 @@ js_SetContextThread(JSContext *cx) memset(&thread->propertyCache, 0, sizeof(thread->propertyCache)); } - /* Assert that the previous cx->thread called JS_ClearContextThread(). */ - JS_ASSERT(!cx->thread || cx->thread == thread); - if (!cx->thread) - JS_APPEND_LINK(&cx->threadLinks, &thread->contextList); + JS_APPEND_LINK(&cx->threadLinks, &thread->contextList); cx->thread = thread; - return JS_TRUE; -} - -/* Remove the owning thread info of a context. */ -void -js_ClearContextThread(JSContext *cx) -{ - /* - * If cx is associated with a thread, this must be called only from that - * thread. If not, this is a harmless no-op. - */ - JS_ASSERT(cx->thread == js_GetCurrentThread(cx->runtime) || !cx->thread); - JS_REMOVE_AND_INIT_LINK(&cx->threadLinks); -#ifdef DEBUG - if (JS_CLIST_IS_EMPTY(&cx->thread->contextList)) { - memset(cx->thread->gcFreeLists, JS_FREE_PATTERN, - sizeof(cx->thread->gcFreeLists)); - } -#endif - cx->thread = NULL; } #endif /* JS_THREADSAFE */ @@ -229,6 +202,12 @@ js_NewContext(JSRuntime *rt, size_t stackChunkSize) JSContext *cx; JSBool ok, first; JSContextCallback cxCallback; +#ifdef JS_THREADSAFE + JSThread *thread = js_GetCurrentThread(rt); + + if (!thread) + return NULL; +#endif cx = (JSContext *) malloc(sizeof *cx); if (!cx) @@ -243,8 +222,12 @@ js_NewContext(JSRuntime *rt, size_t stackChunkSize) #endif cx->scriptStackQuota = JS_DEFAULT_SCRIPT_STACK_QUOTA; #ifdef JS_THREADSAFE - JS_INIT_CLIST(&cx->threadLinks); - js_SetContextThread(cx); + + /* + * At this point cx is not on rt->contextList. Thus we do not need to + * prevent a race against the GC when adding cx to JSThread.contextList. + */ + js_InitContextThread(cx, thread); #endif JS_LOCK_GC(rt); @@ -336,6 +319,9 @@ js_DestroyContext(JSContext *cx, JSDestroyContextMode mode) JSLocalRootStack *lrs; JSLocalRootChunk *lrc; +#ifdef JS_THREADSAFE + JS_ASSERT(CURRENT_THREAD_IS_ME(cx->thread)); +#endif rt = cx->runtime; if (mode != JSDCM_NEW_FAILED) { @@ -400,10 +386,6 @@ js_DestroyContext(JSContext *cx, JSDestroyContextMode mode) * js_DestroyContext that was not last might be waiting in the GC for our * request to end. We'll let it run below, just before we do the truly * final GC and then free atom state. - * - * At this point, cx must be inaccessible to other threads. It's off the - * rt->contextList, and it should not be reachable via any object private - * data structure. */ while (cx->requestDepth != 0) JS_EndRequest(cx); @@ -463,7 +445,13 @@ js_DestroyContext(JSContext *cx, JSDestroyContextMode mode) } #ifdef JS_THREADSAFE - js_ClearContextThread(cx); + /* + * Since cx is not on rt->contextList, it cannot be accessed by the GC + * running on another thread. Thus, compared with JS_ClearContextThread, + * we can safely unlink cx from from JSThread.contextList without taking + * the GC lock. + */ + JS_REMOVE_LINK(&cx->threadLinks); #endif /* Finally, free cx itself. */ diff --git a/mozilla/js/src/jscntxt.h b/mozilla/js/src/jscntxt.h index 0c9805e958a..991f76b65c1 100644 --- a/mozilla/js/src/jscntxt.h +++ b/mozilla/js/src/jscntxt.h @@ -134,11 +134,8 @@ struct JSThread { extern void JS_DLL_CALLBACK js_ThreadDestructorCB(void *ptr); -extern JSBool -js_SetContextThread(JSContext *cx); - extern void -js_ClearContextThread(JSContext *cx); +js_InitContextThread(JSContext *cx, JSThread *thread); extern JSThread * js_GetCurrentThread(JSRuntime *rt); diff --git a/mozilla/js/src/jsgc.c b/mozilla/js/src/jsgc.c index 38325cb63fd..d39a5c27b45 100644 --- a/mozilla/js/src/jsgc.c +++ b/mozilla/js/src/jsgc.c @@ -1454,22 +1454,9 @@ js_AddRootRT(JSRuntime *rt, void *rp, const char *name) * properly with a racing GC, without calling JS_AddRoot from a request. * We have to preserve API compatibility here, now that we avoid holding * rt->gcLock across the mark phase (including the root hashtable mark). - * - * If the GC is running and we're called on another thread, wait for this - * GC activation to finish. We can safely wait here (in the case where we - * are called within a request on another thread's context) without fear - * of deadlock because the GC doesn't set rt->gcRunning until after it has - * waited for all active requests to end. */ JS_LOCK_GC(rt); -#ifdef JS_THREADSAFE - JS_ASSERT(!rt->gcRunning || rt->gcLevel > 0); - if (rt->gcRunning && rt->gcThread->id != js_CurrentThreadId()) { - do { - JS_AWAIT_GC_DONE(rt); - } while (rt->gcLevel > 0); - } -#endif + js_WaitForGC(rt); rhe = (JSGCRootHashEntry *) JS_DHashTableOperate(&rt->gcRootsHash, rp, JS_DHASH_ADD); if (rhe) { @@ -1491,14 +1478,7 @@ js_RemoveRoot(JSRuntime *rt, void *rp) * Same synchronization drill as above in js_AddRoot. */ JS_LOCK_GC(rt); -#ifdef JS_THREADSAFE - JS_ASSERT(!rt->gcRunning || rt->gcLevel > 0); - if (rt->gcRunning && rt->gcThread->id != js_CurrentThreadId()) { - do { - JS_AWAIT_GC_DONE(rt); - } while (rt->gcLevel > 0); - } -#endif + js_WaitForGC(rt); (void) JS_DHashTableOperate(&rt->gcRootsHash, rp, JS_DHASH_REMOVE); rt->gcPoke = JS_TRUE; JS_UNLOCK_GC(rt); @@ -3549,6 +3529,31 @@ js_GC(JSContext *cx, JSGCInvocationKind gckind) } } +#ifdef JS_THREADSAFE + +/* + * If the GC is running and we're called on another thread, wait for this GC + * activation to finish. We can safely wait here without fear of deadlock (in + * the case where we are called within a request on another thread's context) + * because the GC doesn't set rt->gcRunning until after it has waited for all + * active requests to end. + * + * We call here js_CurrentThreadId() after checking for rt->gcRunning to avoid + * expensive calls when the GC is not running. + */ +void +js_WaitForGC(JSRuntime *rt) +{ + JS_ASSERT_IF(rt->gcRunning, rt->gcLevel > 0); + if (rt->gcRunning && rt->gcThread->id != js_CurrentThreadId()) { + do { + JS_AWAIT_GC_DONE(rt); + } while (rt->gcRunning); + } +} + +#endif + void js_UpdateMallocCounter(JSContext *cx, size_t nbytes) { diff --git a/mozilla/js/src/jsgc.h b/mozilla/js/src/jsgc.h index 46351d09dde..12fddb23464 100644 --- a/mozilla/js/src/jsgc.h +++ b/mozilla/js/src/jsgc.h @@ -296,6 +296,20 @@ typedef enum JSGCInvocationKind { extern void js_GC(JSContext *cx, JSGCInvocationKind gckind); + +/* + * This function must be called with the GC lock held. It is a helper for code + * that can potentially run outside JS request to ensure that the GC is not + * running when the function returns. + */ +#ifdef JS_THREADSAFE +extern void +js_WaitForGC(JSRuntime *rt); +#else +# define js_WaitForGC(rt) ((void) 0) +#endif + + /* Call this after succesful malloc of memory for GC-related things. */ extern void js_UpdateMallocCounter(JSContext *cx, size_t nbytes);