There may (or may not) be a race on JSContext::requestDepth. This is x86_64 Linux, m-c, release build, browser startup. The two source locations are JS_FRIEND_API(JSContext *) js_NextActiveContext(JSRuntime *rt, JSContext *cx) { JSContext *iter = cx; #ifdef JS_THREADSAFE while ((cx = js_ContextIterator(rt, JS_FALSE, &iter)) != NULL) { if (cx->requestDepth) <<----------------------- HERE break; } return cx; #else return js_ContextIterator(rt, JS_FALSE, &iter); #endif } JS_PUBLIC_API(void) JS_BeginRequest(JSContext *cx) { #ifdef JS_THREADSAFE JS_ASSERT(CURRENT_THREAD_IS_ME(cx->thread)); if (!cx->requestDepth) { JSRuntime *rt = cx->runtime; JS_LOCK_GC(rt); /* Wait until the GC is finished. */ if (rt->gcThread != cx->thread) { while (rt->gcLevel > 0) JS_AWAIT_GC_DONE(rt); } /* Indicate that a request is running. */ rt->requestCount++; cx->requestDepth = 1; cx->outstandingRequests++; JS_UNLOCK_GC(rt); return; } cx->requestDepth++; <<--------------------------- HERE cx->outstandingRequests++; #endif } The Helgrind complaint giving rise to this enquiry is shown below. Here are some comments from jorendorff on #jsapi: [19:51] <jorendorff> sewardj: I think the intended scheme here is that requestDepth may only transition from 0 to nonzero within the lock; and can only be changed otherwise by cx->thread while a request is held (i.e. nonzero to nonzero) [19:52] <jorendorff> s/within the lock;/within the lock while GC is not happening;/ [19:53] <jorendorff> and readers must either hold the gc lock or must be the GC thread [19:56] <jorendorff> Actually, if the ActiveContext stuff is only called during GC, it might be ok (boggle) [19:59] <jorendorff> Previously I've only thought of four regions, "in a request", "holding gcLock", "I am the GC thread", and "none of the above" [19:59] <jorendorff> and the scheme "can read when in a request or holding gcLock; can read/write when holding gcLock or I am the GC thread" is safe. [20:00] <jorendorff> But demonstrating this is safe requires considering a fifth region, "I am going to be the GC thread and GC is pending" Possible data race during read of size 4 at 0x10205358 by thread #5 at 0x6B1EA7A: js_NextActiveContext(JSRuntime*, JSContext*) (jscntxt.cpp:909) by 0x5561A7D: XPCJSRuntime::WatchdogMain(void*) (xpcjsruntime.cpp:812) by 0x7679E22: _pt_root (ptthread.c:230) by 0x4C2A5D8: mythread_wrapper (hg_intercepts.c:213) by 0x4E34A03: start_thread (pthread_create.c:300) by 0xAAAA80C: clone (clone.S:112) This conflicts with a previous write of size 4 by thread #1 at 0x6B1044D: JS_BeginRequest (jsapi.cpp:792) by 0x5544F0C: nsXPConnect::GetWrapperForObject(JSContext*, JSObject*, JSObject*, nsIPrincipal*, unsigned int, long*) (jsapi.h:569) by 0x557ABF0: XPC_WN_JSOp_ThisObject(JSContext*, JSObject*) (xpcwrappednativejsops.cpp:1538) by 0x6B56DCF: js_ComputeThis (jsobj.h:424) by 0x6B57D9C: js_Invoke (jsinterp.cpp:1218) by 0x6B4829F: js_Interpret (jsops.cpp:2277) by 0x6B58217: js_Invoke (jsinterp.cpp:1378) by 0x6B58787: js_InternalInvoke (jsinterp.cpp:1435) Address 0x10205358 is 856 bytes inside a block of size 1624 alloc'd at 0x4C24E2C: calloc (vg_replace_malloc.c:467) by 0x6B1FFF3: js_NewContext(JSRuntime*, unsigned long) (jsutil.h:191) by 0x55E3FDD: mozJSComponentLoader::ReallyInit() (mozJSComponentLoader.cpp:595) by 0x55E4E91: mozJSComponentLoader::LoadModule(nsILocalFile*, nsIModule**) (mozJSComponentLoader.cpp:680) by 0x610FCAF: nsFactoryEntry::GetFactory(nsIFactory**) (nsComponentManager.cpp:3676) by 0x610FE18: nsComponentManagerImpl::CreateInstanceByContractID(char const*, nsISupports*, nsID const&, void**) (nsComponentManager.cpp:1680) by 0x610F190: nsComponentManagerImpl::GetServiceByContractID(char const*, nsID const&, void**) (nsComponentManager.cpp:2252) by 0x60D4D1B: nsGetServiceByContractIDWithError::operator()(nsID const&, void**) const (nsComponentManagerUtils.cpp:288) by 0x60D4067: nsCOMPtr_base::assign_from_gs_contractid_with_error(nsGetServiceByContractIDWithError const&, nsID const&) (nsCOMPtr.cpp:141) by 0x5DF083E: nsAppStartupNotifier::Observe(nsISupports*, char const*, unsigned short const*) (nsCOMPtr.h:1031) by 0x5538009: XRE_main (nsAppRunner.cpp:3376) by 0x400EBB: main (nsBrowserApp.cpp:158)

Dear everyone: please don't mark this RESO INVALID, not that I think you were going to -- either there's a race, or the rules protecting JSContext::requestDepth are unobvious and not commented (at least, not at its declaration, and not anywhere that I was able to find). Either one is a bug.

/* * Iterate through contexts with active requests. The caller must be holding * rt->gcLock in case of a thread-safe build, or otherwise guarantee that the * context list is not alternated asynchroniously. */ extern JS_FRIEND_API(JSContext *) js_NextActiveContext(JSRuntime *, JSContext *); s/alternated/altered/ But of course this doesn't help the unlocked cx->requestDepth++ in JS_BeginRequest. The race to increment from >= 1 to >= 2 is certainly wide open, but JS_BeginRequest does lock on 0 -> 1, and of course JS_EndRequest gets the gcLock going from 1 -> 0. So this seems ok. /me whistles past graveyard... /be