Bug 429442 - "crashes [@ nsJSIID::HasInstance][@ XPCNativeSet::FindInterfaceWithIID]". r+sr=jst, a=beltzner.

dom/src/base/nsDOMClassInfo.cpp

@@ -8872,11 +8872,22 @@ public:
 
   NS_IMETHOD Run()
   {
+    JSContext* cx = nsnull;
+    if (mContext) {
+      cx = (JSContext*)mContext->GetNativeContext();
+    } else {
+      nsCOMPtr<nsIThreadJSContextStack> stack =
+        do_GetService("@mozilla.org/js/xpc/ContextStack;1");
+      NS_ENSURE_TRUE(stack, NS_OK);
+
+      stack->GetSafeJSContext(&cx);
+      NS_ENSURE_TRUE(cx, NS_OK);
+    }
+
     JSObject* obj = nsnull;
     mWrapper->GetJSObject(&obj);
     NS_ASSERTION(obj, "Should never be null");
-    nsHTMLPluginObjElementSH::SetupProtoChain(
-      mWrapper, (JSContext*)mContext->GetNativeContext(), obj);
+    nsHTMLPluginObjElementSH::SetupProtoChain(mWrapper, cx, obj);
     return NS_OK;
   }
 
@@ -9024,15 +9035,24 @@ nsHTMLPluginObjElementSH::PostCreate(nsIXPConnectWrappedNative *wrapper,
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (nsContentUtils::IsSafeToRunScript()) {
-    return SetupProtoChain(wrapper, cx, obj);
+    rv = SetupProtoChain(wrapper, cx, obj);
+
+    // If SetupProtoChain failed then we're in real trouble. We're about to fail
+    // PostCreate but it's more than likely that we handed our (now invalid)
+    // wrapper to someone already. Bug 429442 is an example of the kind of crash
+    // that can result from such a situation. We'll return NS_OK for the time
+    // being and hope for the best.
+    NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "SetupProtoChain failed!");
+    return NS_OK;
   }
 
-  nsCOMPtr<nsIScriptContext> scriptContext =
-    GetScriptContextFromJSContext(cx);
-  NS_ENSURE_TRUE(scriptContext, NS_ERROR_UNEXPECTED);
+  // This may be null if the JS context is not a DOM context. That's ok, we'll
+  // use the safe context from XPConnect in the runnable.
+  nsCOMPtr<nsIScriptContext> scriptContext = GetScriptContextFromJSContext(cx);
 
-  nsContentUtils::AddScriptRunner(
-      new nsPluginProtoChainInstallRunner(wrapper, scriptContext));
+  nsRefPtr<nsPluginProtoChainInstallRunner> runner =
+    new nsPluginProtoChainInstallRunner(wrapper, scriptContext);
+  nsContentUtils::AddScriptRunner(runner);
 
   return NS_OK;
 }

js/src/xpconnect/src/xpcmaps.h

@@ -204,6 +204,13 @@ public:
     inline void Remove(XPCWrappedNative* wrapper)
     {
         NS_PRECONDITION(wrapper,"bad param");
+#ifdef DEBUG
+        XPCWrappedNative* wrapperInMap = Find(wrapper->GetIdentityObject());
+        NS_ASSERTION(!wrapperInMap || wrapperInMap == wrapper,
+                     "About to remove a different wrapper with the same "
+                     "nsISupports identity! This will most likely cause serious "
+                     "problems!");
+#endif
         JS_DHashTableOperate(mTable, wrapper->GetIdentityObject(), JS_DHASH_REMOVE);
     }
 

js/src/xpconnect/src/xpcwrappednative.cpp

@@ -546,11 +546,27 @@ XPCWrappedNative::GetNewOrUsed(XPCCallContext& ccx,
                      PostCreate(wrapper, ccx, wrapper->GetFlatJSObject());
             if(NS_FAILED(rv))
             {
+                // PostCreate failed and that's Very Bad. We'll remove it from
+                // the map and mark it as invalid, but the PostCreate function
+                // may have handed the partially-constructed-and-now-invalid
+                // wrapper to someone before failing. Or, perhaps worse, the
+                // PostCreate call could have triggered code that reentered
+                // XPConnect and tried to wrap the same object. In that case
+                // *we* hand out the invalid wrapper since it is already in our
+                // map :(
+                NS_ERROR("PostCreate failed! This is known to cause "
+                         "inconsistent state for some class types and may even "
+                         "cause a crash in combination with a JS GC. Fix the "
+                         "failing PostCreate ASAP!");
+
                 {   // scoped lock
                     XPCAutoLock lock(mapLock);
                     map->Remove(wrapper);
                 }
 
+                // This would be a good place to tell the wrapper not to remove
+                // itself from the map when it dies... See bug 429442.
+
                 wrapper->Release();
                 return rv;
             }
@@ -671,6 +687,9 @@ XPCWrappedNative::~XPCWrappedNative()
 
         // scoped lock
         XPCAutoLock lock(GetRuntime()->GetMapLock());
+
+        // Post-1.9 we should not remove this wrapper from the map if it is
+        // uninitialized.
         map->Remove(this);
     }