Bug 180380. nsXPCComponents object and its wrapper leaked at shutdown. Patch by David Baron, David Bradley, and Robert Sayre. r=jst/bzbarsky, sr=dbaron

js/src/xpconnect/src/nsXPConnect.cpp

@@ -217,11 +217,6 @@ struct JSObjectRefcounts
 
 nsXPConnect::~nsXPConnect()
 {
-    // XXX It would be nice if we could get away with doing a GC here and also
-    // calling Release on the natives no longer reachable via XPConnect. As
-    // noted all over the place, this makes bad things happen since shutdown is
-    // an unstable time for so many modules who have not planned well for it.
-
     NS_ASSERTION(!mCycleCollectionContext,
                  "Didn't call FinishCycleCollection?");
     nsCycleCollector_forgetRuntime(nsIProgrammingLanguage::JAVASCRIPT);
@@ -231,36 +226,31 @@ nsXPConnect::~nsXPConnect()
         mObjRefcounts = NULL;
     }
 
+    JSContext *cx = nsnull;
+    if (mRuntime) {
+        cx = JS_NewContext(mRuntime->GetJSRuntime(), 8192);
+    }
+
+    XPCPerThreadData::CleanupAllThreads();
     mShuttingDown = JS_TRUE;
-    { // scoped callcontext
-        XPCCallContext ccx(NATIVE_CALLER);
-        if(ccx.IsValid())
-        {
-            XPCWrappedNativeScope::SystemIsBeingShutDown(ccx);
-            if(mRuntime)
-                mRuntime->SystemIsBeingShutDown(&ccx);
-                
-        }
+    if (cx) {
+        // Create our own JSContext rather than an XPCCallContext, since
+        // otherwise we will create a new safe JS context and attach a
+        // components object that won't get GCed.
+        JS_BeginRequest(cx);
+
+        // XXX Call even if |mRuntime| null?
+        XPCWrappedNativeScope::SystemIsBeingShutDown(cx);
+
+        mRuntime->SystemIsBeingShutDown(cx);
+
+        JS_EndRequest(cx);
+        JS_DestroyContext(cx);
     }
 
     NS_IF_RELEASE(mContextStack);
     NS_IF_RELEASE(mDefaultSecurityManager);
 
-    // Unfortunately calling CleanupAllThreads before the stuff above
-    // (esp. SystemIsBeingShutDown) causes too many bad things to happen
-    // as the Release calls propagate. See the comment in this function in
-    // revision 1.35 of this file.
-    //
-    // I filed a bug on xpcom regarding the bad things that happen
-    // if people try to create components during shutdown.
-    // http://bugzilla.mozilla.org/show_bug.cgi?id=37058
-    //
-    // Also, we just plain need the context stack for at least the current
-    // thread to be in place. Unfortunately, this will leak stuff on the
-    // stacks' safeJSContexts. But, this is a shutdown leak only.
-
-    XPCPerThreadData::CleanupAllThreads();
-
     // shutdown the logging system
     XPC_LOG_FINISH();
 

js/src/xpconnect/src/xpcjsruntime.cpp

@@ -650,15 +650,15 @@ DetachedWrappedNativeProtoShutdownMarker(JSDHashTable *table, JSDHashEntryHdr *h
     XPCWrappedNativeProto* proto = 
         (XPCWrappedNativeProto*)((JSDHashEntryStub*)hdr)->key;
 
-    proto->SystemIsBeingShutDown(*((XPCCallContext*)arg));
+    proto->SystemIsBeingShutDown((JSContext*)arg);
     return JS_DHASH_NEXT;
 }
 
-void XPCJSRuntime::SystemIsBeingShutDown(XPCCallContext* ccx)
+void XPCJSRuntime::SystemIsBeingShutDown(JSContext* cx)
 {
     if(mDetachedWrappedNativeProtoMap)
         mDetachedWrappedNativeProtoMap->
-            Enumerate(DetachedWrappedNativeProtoShutdownMarker, ccx);
+            Enumerate(DetachedWrappedNativeProtoShutdownMarker, cx);
 }
 
 XPCJSRuntime::~XPCJSRuntime()

js/src/xpconnect/src/xpcprivate.h

@@ -672,7 +672,7 @@ public:
 
     void DebugDump(PRInt16 depth);
 
-    void SystemIsBeingShutDown(XPCCallContext* ccx);
+    void SystemIsBeingShutDown(JSContext* cx);
 
     PRThread* GetThreadRunningGC() const {return mThreadRunningGC;}
 
@@ -1130,7 +1130,7 @@ public:
                         JSBool OKIfNotInitialized = JS_FALSE);
 
     static void
-    SystemIsBeingShutDown(XPCCallContext& ccx);
+    SystemIsBeingShutDown(JSContext* cx);
 
     static void
     TraceJS(JSTracer* trc, XPCJSRuntime* rt);
@@ -1768,7 +1768,7 @@ public:
 
     void JSProtoObjectFinalized(JSContext *cx, JSObject *obj);
 
-    void SystemIsBeingShutDown(XPCCallContext& ccx);
+    void SystemIsBeingShutDown(JSContext* cx);
 
     void DebugDump(PRInt16 depth);
 
@@ -2026,7 +2026,7 @@ public:
 
     void FlatJSObjectFinalized(JSContext *cx, JSObject *obj);
 
-    void SystemIsBeingShutDown(XPCCallContext& ccx);
+    void SystemIsBeingShutDown(JSContext* cx);
 
 #ifdef XPC_DETECT_LEADING_UPPERCASE_ACCESS_ERRORS
     // This will try to find a member that is of the form "camelCased"

js/src/xpconnect/src/xpcthreadcontext.cpp

@@ -448,6 +448,8 @@ GetThreadStackLimit()
   return stackLimit;
 }
 
+MOZ_DECL_CTOR_COUNTER(xpcPerThreadData)
+
 XPCPerThreadData::XPCPerThreadData()
     :   mJSContextStack(new XPCJSContextStack()),
         mNextThread(nsnull),
@@ -465,6 +467,7 @@ XPCPerThreadData::XPCPerThreadData()
       , mWrappedNativeThreadsafetyReportDepth(0)
 #endif
 {
+    MOZ_COUNT_CTOR(xpcPerThreadData);
     if(gLock)
     {
         nsAutoLock lock(gLock);
@@ -489,6 +492,8 @@ XPCPerThreadData::Cleanup()
 
 XPCPerThreadData::~XPCPerThreadData()
 {
+    MOZ_COUNT_DTOR(xpcPerThreadData);
+
     Cleanup();
 
     // Unlink 'this' from the list of threads.

js/src/xpconnect/src/xpcwrappednative.cpp

@@ -1017,8 +1017,21 @@ XPCWrappedNative::FlatJSObjectFinalized(JSContext *cx, JSObject *obj)
 }
 
 void
-XPCWrappedNative::SystemIsBeingShutDown(XPCCallContext& ccx)
+XPCWrappedNative::SystemIsBeingShutDown(JSContext* cx)
 {
+#ifdef DEBUG_xpc_hacker
+    {
+        printf("Removing root for still-live XPCWrappedNative %p wrapping:\n",
+               NS_STATIC_CAST(void*, this));
+        for(PRUint16 i = 0, i_end = mSet->GetInterfaceCount(); i < i_end; ++i)
+        {
+            nsXPIDLCString name;
+            mSet->GetInterfaceAt(i)->GetInterfaceInfo()
+                ->GetName(getter_Copies(name));
+            printf("  %s\n", name.get());
+        }
+    }
+#endif
     DEBUG_TrackShutdownWrapper(this);
 
     if(!IsValid())
@@ -1031,13 +1044,13 @@ XPCWrappedNative::SystemIsBeingShutDown(XPCCallContext& ccx)
     // We leak mIdentity (see above).
 
     // short circuit future finalization
-    JS_SetPrivate(ccx, mFlatJSObject, nsnull);
+    JS_SetPrivate(cx, mFlatJSObject, nsnull);
     mFlatJSObject = nsnull; // This makes 'IsValid()' return false.
 
     XPCWrappedNativeProto* proto = GetProto();
 
     if(HasProto())
-        proto->SystemIsBeingShutDown(ccx);
+        proto->SystemIsBeingShutDown(cx);
 
     if(mScriptableInfo &&
        (!HasProto() ||
@@ -1056,7 +1069,7 @@ XPCWrappedNative::SystemIsBeingShutDown(XPCCallContext& ccx)
         {
             if(to->GetJSObject())
             {
-                JS_SetPrivate(ccx, to->GetJSObject(), nsnull);
+                JS_SetPrivate(cx, to->GetJSObject(), nsnull);
 #ifdef XPC_IDISPATCH_SUPPORT
                 if(to->IsIDispatch())
                     delete to->GetIDispatchInfo();

js/src/xpconnect/src/xpcwrappednativeproto.cpp

@@ -144,7 +144,7 @@ XPCWrappedNativeProto::JSProtoObjectFinalized(JSContext *cx, JSObject *obj)
 }
 
 void
-XPCWrappedNativeProto::SystemIsBeingShutDown(XPCCallContext& ccx)
+XPCWrappedNativeProto::SystemIsBeingShutDown(JSContext* cx)
 {
     // Note that the instance might receive this call multiple times
     // as we walk to here from various places.
@@ -162,7 +162,7 @@ XPCWrappedNativeProto::SystemIsBeingShutDown(XPCCallContext& ccx)
     if(mJSProtoObject)
     {
         // short circuit future finalization
-        JS_SetPrivate(ccx, mJSProtoObject, nsnull);
+        JS_SetPrivate(cx, mJSProtoObject, nsnull);
         mJSProtoObject = nsnull;
     }
 }

js/src/xpconnect/src/xpcwrappednativescope.cpp

@@ -485,10 +485,10 @@ XPCWrappedNativeScope::KillDyingScopes()
 
 struct ShutdownData
 {
-    ShutdownData(XPCCallContext& accx)
-        : ccx(accx), wrapperCount(0),
+    ShutdownData(JSContext* acx)
+        : cx(acx), wrapperCount(0),
           sharedProtoCount(0), nonSharedProtoCount(0) {}
-    XPCCallContext& ccx;
+    JSContext* cx;
     int wrapperCount;
     int sharedProtoCount;
     int nonSharedProtoCount;
@@ -505,7 +505,7 @@ WrappedNativeShutdownEnumerator(JSDHashTable *table, JSDHashEntryHdr *hdr,
     {
         if(wrapper->HasProto() && !wrapper->HasSharedProto())
             data->nonSharedProtoCount++;
-        wrapper->SystemIsBeingShutDown(data->ccx);
+        wrapper->SystemIsBeingShutDown(data->cx);
         data->wrapperCount++;
     }
     return JS_DHASH_REMOVE;
@@ -517,21 +517,21 @@ WrappedNativeProtoShutdownEnumerator(JSDHashTable *table, JSDHashEntryHdr *hdr,
 {
     ShutdownData* data = (ShutdownData*) arg;
     ((ClassInfo2WrappedNativeProtoMap::Entry*)hdr)->value->
-        SystemIsBeingShutDown(data->ccx);
+        SystemIsBeingShutDown(data->cx);
     data->sharedProtoCount++;
     return JS_DHASH_REMOVE;
 }
 
 //static
 void
-XPCWrappedNativeScope::SystemIsBeingShutDown(XPCCallContext& ccx)
+XPCWrappedNativeScope::SystemIsBeingShutDown(JSContext* cx)
 {
     DEBUG_TrackScopeTraversal();
     DEBUG_TrackScopeShutdown();
 
     int liveScopeCount = 0;
 
-    ShutdownData data(ccx);
+    ShutdownData data(cx);
 
     XPCWrappedNativeScope* cur;