fix for crash that can happen when wrapped JS objects get accessed after xpconnect shutdown. This was burning outside users of xpconnect. bug 45669. r=mccabe@netscape.com a=brendan@mozilla.org

js/src/xpconnect/shell/xpcshell.cpp

@@ -19,6 +19,7 @@
  * Rights Reserved.
  *
  * Contributor(s): 
+ *   John Bandhauer <jband@netscape.com>
  *   Pierre Phaneuf <pp@ludusdesign.com>
  *
  * Alternatively, the contents of this file may be used under the
@@ -765,6 +766,16 @@ main(int argc, char **argv)
 
     result = ProcessArgs(jscontext, glob, argv, argc);
 
+
+//#define TEST_CALL_ON_WRAPPED_JS_AFTER_SHUTDOWN 1
+
+#ifdef TEST_CALL_ON_WRAPPED_JS_AFTER_SHUTDOWN
+    // test of late call and release (see below)
+    nsCOMPtr<nsIJSContextStack> bogus;
+    xpc->WrapJS(jscontext, glob, NS_GET_IID(nsIJSContextStack), 
+                (void**) getter_AddRefs(bogus));
+#endif
+
     JS_ClearScope(jscontext, glob);
     js_ForceGC(jscontext);
     JSContext *oldcx;
@@ -780,6 +791,13 @@ main(int argc, char **argv)
     rv = NS_ShutdownXPCOM( NULL );
     NS_ASSERTION(NS_SUCCEEDED(rv), "NS_ShutdownXPCOM failed");
 
+#ifdef TEST_CALL_ON_WRAPPED_JS_AFTER_SHUTDOWN
+    // test of late call and release (see above)
+    JSContext* bogusCX;
+    bogus->Peek(&bogusCX);
+    bogus = nsnull;
+#endif
+
     return result;
 }
 

js/src/xpconnect/src/xpcjsruntime.cpp

@@ -61,12 +61,22 @@ hash_root(const void *key)
 JS_STATIC_DLL_CALLBACK(intN)
 DEBUG_WrapperChecker(JSHashEntry *he, intN i, void *arg)
 {
-    NS_ASSERTION(!((nsXPCWrappedNative*)he->value)->IsValid(), "found a 'valid' wrappper!");
+    NS_ASSERTION(!((nsXPCWrappedNative*)he->value)->IsValid(), "found a 'valid' wrapper!");
     ++ *((int*)arg);
     return HT_ENUMERATE_NEXT;
 }
 #endif
 
+JS_STATIC_DLL_CALLBACK(intN)
+WrappedJSShutdownMarker(JSHashEntry *he, intN i, void *arg)
+{
+    nsXPCWrappedJS* wrapper = (nsXPCWrappedJS*)he->value;
+    NS_ASSERTION(wrapper, "found a null JS wrapper!");
+    NS_ASSERTION(wrapper->IsValid(), "found an invalid JS wrapper!");
+    wrapper->SystemIsBeingShutDown();
+    return HT_ENUMERATE_NEXT;
+}
+
 XPCJSRuntime::~XPCJSRuntime()
 {
 #ifdef XPC_DUMP_AT_SHUTDOWN
@@ -96,6 +106,7 @@ XPCJSRuntime::~XPCJSRuntime()
         if(count)
             printf("deleting XPCJSRuntime with %d live wrapped JSObject\n", (int)count);        
 #endif
+        mWrappedJSMap->Enumerate(WrappedJSShutdownMarker, nsnull); 
         delete mWrappedJSMap;
     }
 

js/src/xpconnect/src/xpcprivate.h

@@ -771,6 +771,9 @@ public:
 
     nsXPCWrappedJS* Find(REFNSIID aIID);
 
+    JSBool IsValid() const {return mJSObj != nsnull;}
+    void SystemIsBeingShutDown();
+
     virtual ~nsXPCWrappedJS();
 private:
     nsXPCWrappedJS();   // not implemented

js/src/xpconnect/src/xpcwrappedjs.cpp

@@ -43,6 +43,9 @@
 NS_IMETHODIMP
 nsXPCWrappedJS::QueryInterface(REFNSIID aIID, void** aInstancePtr)
 {
+    if(!IsValid())
+        return NS_ERROR_UNEXPECTED;
+
     if(nsnull == aInstancePtr)
     {
         NS_PRECONDITION(0, "null pointer");
@@ -250,7 +253,8 @@ nsXPCWrappedJS::nsXPCWrappedJS(XPCContext* xpcc,
 nsXPCWrappedJS::~nsXPCWrappedJS()
 {
     NS_PRECONDITION(0 == mRefCnt, "refcounting error");
-    if(mClass)
+    // Any destructors called after shutdown are just going to leak stuff.
+    if(IsValid())
     {
         XPCJSRuntime* rt = nsXPConnect::GetRuntime();
         if(rt)
@@ -266,7 +270,7 @@ nsXPCWrappedJS::~nsXPCWrappedJS()
             }
                 JS_RemoveRootRT(rt->GetJSRuntime(), &mJSObj);
         }
-        NS_RELEASE(mClass);
+        NS_IF_RELEASE(mClass);
     }
     if(mNext)
         NS_DELETEXPCOM(mNext);  // cascaded delete
@@ -295,6 +299,9 @@ nsXPCWrappedJS::GetInterfaceInfo(nsIInterfaceInfo** info)
     NS_ASSERTION(GetClass(), "wrapper without class");
     NS_ASSERTION(GetClass()->GetInterfaceInfo(), "wrapper class without interface");
 
+    // Since failing to get this info will crash some platforms(!), we keep
+    // mClass valid at shutdown time.
+
     if(!(*info = GetClass()->GetInterfaceInfo()))
         return NS_ERROR_UNEXPECTED;
     NS_ADDREF(*info);
@@ -306,6 +313,8 @@ nsXPCWrappedJS::CallMethod(PRUint16 methodIndex,
                            const nsXPTMethodInfo* info,
                            nsXPTCMiniVariant* params)
 {
+    if(!IsValid())
+        return NS_ERROR_UNEXPECTED;
     return GetClass()->CallMethod(this, methodIndex, info, params);
 }
 
@@ -318,6 +327,27 @@ nsXPCWrappedJS::GetIID(nsIID** iid)
     return *iid ? NS_OK : NS_ERROR_UNEXPECTED;
 }
 
+void 
+nsXPCWrappedJS::SystemIsBeingShutDown()
+{
+    // XXX It turns out that it is better to leak here then to do any Releases 
+    // and have them propagate into all sorts of mischief as the system is being
+    // shutdown. This was learned the hard way :(    
+    
+    // mJSObj == nsnull is used to indicate that the wrapper is no longer valid
+    // and that calls should fail without trying to use any of the 
+    // xpconnect mechanisms. 'IsValid' is implemented by checking this pointer.
+
+    // NOTE: that mClass is retained so that GetInterfaceInfo can continue to 
+    // work (and avoid crashing some platforms).
+
+    mJSObj = nsnull;
+    
+    // Notify other wrappers in the chain.
+    if(mNext)
+        mNext->SystemIsBeingShutDown();
+}
+
 /***************************************************************************/
 
 NS_IMETHODIMP

js/src/xpconnect/src/xpcwrappednative.cpp

@@ -420,7 +420,7 @@ nsXPCWrappedNative::Find(REFNSIID aIID)
 void 
 nsXPCWrappedNative::SystemIsBeingShutDown()
 {
-    // XXX It turns out that it is better to leak here then to do any Release's 
+    // XXX It turns out that it is better to leak here then to do any Releases 
     // and have them propagate into all sorts of mischief as the system is being
     // shutdown. This was learned the hard way :(    
     

js/src/xpconnect/src/xpcwrappednativeclass.cpp

@@ -1283,9 +1283,20 @@ nsXPCWrappedNativeClass::GetWrappedNativeOfJSObject(JSContext* cx,
                                                     JSObject* jsobj)
 {
     NS_PRECONDITION(jsobj, "bad param");
-    if(jsobj && (JS_InstanceOf(cx, jsobj, &WrappedNative_class, nsnull) ||
-                 JS_InstanceOf(cx, jsobj, &WrappedNativeWithCall_class, nsnull)))
-        return (nsXPCWrappedNative*) JS_GetPrivate(cx, jsobj);
+    JSObject* cur = jsobj;
+
+    while(cur)
+    {
+        if(JS_InstanceOf(cx, cur, &WrappedNative_class, nsnull) ||
+           JS_InstanceOf(cx, cur, &WrappedNativeWithCall_class, nsnull))
+            return (nsXPCWrappedNative*) JS_GetPrivate(cx, cur);
+        // This was an attempt to make it possible to use a wrapped
+        // native as a __proto__ for a plain JS object. There are still 
+        // problems with making this work.
+
+        // cur = JS_GetPrototype(cx, cur);
+        break;
+    }
     return nsnull;
 }
 

js/src/xpconnect/tests/js/old/xpctest_echo.js

@@ -356,6 +356,17 @@ if(all_ok)
 all_ok = echo.SharedString() == "a static string";
 print("[shared] test - "+(all_ok ? "passed" : "failed"));
 
+/***************************************************************************/
+// test wrapper Service Identity
+
+var iface = Components.interfaces["nsIScriptError"];
+var clazz = Components.classes["mozilla.scripterror.1"];
+var foo = clazz.getService(iface);
+var bar = clazz.getService(iface);
+all_ok = foo === bar;
+print("service identity test - "+(all_ok ? "passed" : "failed"));
+foo = bar = iface = clazz = null;
+
 /***************************************************************************/
 // Components object test...
 // print(".......................................");