Bug 340212: All XPC GC tracing now happens in the runtime trace callback.

js/src/xpconnect/src/xpcjsruntime.cpp

@@ -250,8 +250,10 @@ ContextCallback(JSContext *cx, uintN operation)
 }
 
 // static
-void XPCJSRuntime::TraceJS(JSTracer *trc, XPCJSRuntime* self)
+void XPCJSRuntime::TraceJS(JSTracer* trc, void* data)
 {
+    XPCJSRuntime* self = (XPCJSRuntime*)data;
+
     // Skip this part if XPConnect is shutting down. We get into
     // bad locking problems with the thread iteration otherwise.
     if(!self->GetXPConnect()->IsShuttingDown())
@@ -272,6 +274,8 @@ void XPCJSRuntime::TraceJS(JSTracer *trc, XPCJSRuntime* self)
             }
         }
     }
+
+    XPCWrappedNativeScope::TraceJS(trc, self);
 }
 
 // static
@@ -303,8 +307,6 @@ JSBool XPCJSRuntime::GCCallback(JSContext *cx, JSGCStatus status)
                     self->mThreadRunningGC = PR_GetCurrentThread();
                 }
 
-                TraceJS(JS_GetGCMarkingTracer(cx), self);
-
                 dyingWrappedJSArray = &self->mWrappedJSToReleaseArray;
                 {
                     XPCLock* lock = self->GetMainThreadOnlyGC() ?
@@ -843,6 +845,7 @@ XPCJSRuntime::XPCJSRuntime(nsXPConnect* aXPConnect,
         gOldJSContextCallback = JS_SetContextCallback(mJSRuntime,
                                                       ContextCallback);
         gOldJSGCCallback = JS_SetGCCallbackRT(mJSRuntime, GCCallback);
+        JS_SetExtraGCRoots(mJSRuntime, TraceJS, this);
     }
 
     // Install a JavaScript 'debugger' keyword handler in debug builds only

js/src/xpconnect/src/xpcprivate.h

@@ -630,7 +630,7 @@ public:
         return mStrings[index];
     }
 
-    static void TraceJS(JSTracer *trc, XPCJSRuntime* self);
+    static void JS_DLL_CALLBACK TraceJS(JSTracer* trc, void* data);
 
     static JSBool JS_DLL_CALLBACK GCCallback(JSContext *cx, JSGCStatus status);
 
@@ -1730,10 +1730,6 @@ public:
 
     void DebugDump(PRInt16 depth);
 
-    // During the mark traversal of JS GC this is called in the 'early' phase
-    // by AutoMarkingWrappedNativeProtoPtr.
-    // 'early' meaning after JSGC_MARK_END and before JSGC_FINALIZE_END.
-    // At this point in time we can still mark JSObjects in the JS gc heap.
     void TraceJS(JSTracer* trc)
     {
         if(mJSProtoObject)

js/src/xpconnect/src/xpcwrappednativescope.cpp

@@ -262,7 +262,7 @@ XPCWrappedNativeScope::~XPCWrappedNativeScope()
 
 
 JS_STATIC_DLL_CALLBACK(JSDHashOperator)
-WrappedNativeJSGCThingMarker(JSDHashTable *table, JSDHashEntryHdr *hdr,
+WrappedNativeJSGCThingTracer(JSDHashTable *table, JSDHashEntryHdr *hdr,
                              uint32 number, void *arg)
 {
     XPCWrappedNative* wrapper = ((Native2WrappedNativeMap::Entry*)hdr)->value;
@@ -287,10 +287,14 @@ WrappedNativeJSGCThingMarker(JSDHashTable *table, JSDHashEntryHdr *hdr,
 void
 XPCWrappedNativeScope::TraceJS(JSTracer* trc, XPCJSRuntime* rt)
 {
+    // FIXME The lock may not be necessary during tracing as that serializes
+    // access to JS runtime. See bug 380139.
+    XPCAutoLock lock(rt->GetMapLock());
+
     // Do JS_CallTracer for all wrapperednatives with external references.
     for(XPCWrappedNativeScope* cur = gScopes; cur; cur = cur->mNext)
     {
-        cur->mWrappedNativeMap->Enumerate(WrappedNativeJSGCThingMarker, trc);
+        cur->mWrappedNativeMap->Enumerate(WrappedNativeJSGCThingTracer, trc);
     }
 }
 
@@ -298,14 +302,15 @@ XPCWrappedNativeScope::TraceJS(JSTracer* trc, XPCJSRuntime* rt)
 void
 XPCWrappedNativeScope::FinishedMarkPhaseOfGC(JSContext* cx, XPCJSRuntime* rt)
 {
-    // Hold the lock until return...
+    // FIXME The lock may not be necessary since we are inside JSGC_MARK_END
+    // callback and GX serializes access to JS runtime. See bug 380139.
     XPCAutoLock lock(rt->GetMapLock());
 
-    TraceJS(JS_GetGCMarkingTracer(cx), rt);
-
-    // Since the JSGC_END call happens outside of a lock,
-    // it is possible for us to get called here twice before the FinshedGC
-    // call happens. So, we allow for gDyingScopes not being null.
+    // We are in JSGC_MARK_END and JSGC_FINALIZE_END must always follow it
+    // calling FinishedFinalizationPhaseOfGC and clearing gDyingScopes in
+    // KillDyingScopes.
+    NS_ASSERTION(gDyingScopes == nsnull,
+                 "JSGC_MARK_END without JSGC_FINALIZE_END");
 
     XPCWrappedNativeScope* prev = nsnull;
     XPCWrappedNativeScope* cur = gScopes;
@@ -354,7 +359,9 @@ XPCWrappedNativeScope::FinishedFinalizationPhaseOfGC(JSContext* cx)
     if(!rt)
         return;
 
-    // Hold the lock until return...
+    // FIXME The lock may not be necessary since we are inside
+    // JSGC_FINALIZE_END callback and at this point GC still serializes access
+    // to JS runtime. See bug 380139.
     XPCAutoLock lock(rt->GetMapLock());
     KillDyingScopes();
 }