Bug 338110 - Remove operation on JavaXPTCStub map is called with bad parameters. XULRunner only. r=bsmedberg.

extensions/java/xpcom/nsJavaWrapper.cpp

@@ -1653,7 +1653,8 @@ CreateJavaProxy(JNIEnv* env, nsISupports* aXPCOMObject, const nsIID& aIID,
 #ifdef DEBUG_JAVAXPCOM
       char* iid_str = aIID.ToString();
       LOG(("+ CreateJavaProxy (Java=%08x | XPCOM=%08x | IID=%s)\n",
-           (PRUint32) env->CallIntMethod(java_obj, hashCodeMID),
+           (PRUint32) env->CallStaticIntMethod(systemClass, hashCodeMID,
+                                               java_obj),
            (PRUint32) aXPCOMObject, iid_str));
       PR_Free(iid_str);
 #endif
@@ -1695,7 +1696,8 @@ GetXPCOMInstFromProxy(JNIEnv* env, jobject aJavaObject, void** aResult)
   inst->InterfaceInfo()->GetInterfaceIID(&iid);
   char* iid_str = iid->ToString();
   LOG(("< GetXPCOMInstFromProxy (Java=%08x | XPCOM=%08x | IID=%s)\n",
-       (PRUint32) env->CallIntMethod(aJavaObject, hashCodeMID),
+       (PRUint32) env->CallStaticIntMethod(systemClass, hashCodeMID,
+                                           aJavaObject),
        (PRUint32) inst->GetInstance(), iid_str));
   PR_Free(iid_str);
   nsMemory::Free(iid);
@@ -1749,7 +1751,9 @@ JAVAPROXY_NATIVE(finalizeProxy) (JNIEnv *env, jclass that, jobject aJavaProxy)
 
 #ifdef DEBUG_JAVAXPCOM
   LOG(("- Finalize (Java=%08x | XPCOM=%08x)\n",
-       (PRUint32) env->CallIntMethod(aJavaProxy, hashCodeMID), xpcom_addr));
+       (PRUint32) env->CallStaticIntMethod(systemClass, hashCodeMID,
+                                           aJavaProxy),
+       xpcom_addr));
 #endif
 }
 

extensions/java/xpcom/nsJavaXPCOMBindingUtils.cpp

@@ -50,6 +50,7 @@
 
 JavaVM* gCachedJVM = nsnull;
 
+jclass systemClass = nsnull;
 jclass booleanClass = nsnull;
 jclass charClass = nsnull;
 jclass byteClass = nsnull;
@@ -145,10 +146,12 @@ InitializeJavaGlobals(JNIEnv *env)
   }
 
   jclass clazz;
-  if (!(clazz = env->FindClass("java/lang/Object")) ||
-      !(hashCodeMID = env->GetMethodID(clazz, "hashCode","()I")))
+  if (!(clazz = env->FindClass("java/lang/System")) ||
+      !(systemClass = (jclass) env->NewGlobalRef(clazz)) ||
+      !(hashCodeMID = env->GetStaticMethodID(clazz, "identityHashCode",
+                                             "(Ljava/lang/Object;)I")))
   {
-    NS_WARNING("Problem creating java.lang.Object globals");
+    NS_WARNING("Problem creating java.lang.System globals");
     goto init_error;
   }
 
@@ -361,6 +364,10 @@ FreeJavaGlobals(JNIEnv* env)
   }
 
   // Free remaining Java globals
+  if (systemClass) {
+    env->DeleteGlobalRef(systemClass);
+    systemClass = nsnull;
+  }
   if (booleanClass) {
     env->DeleteGlobalRef(booleanClass);
     booleanClass = nsnull;
@@ -479,7 +486,8 @@ DestroyJavaProxyMappingEnum(PLDHashTable* aTable, PLDHashEntryHdr* aHeader,
 #ifdef DEBUG_JAVAXPCOM
       char* iid_str = item->iid.ToString();
       LOG(("- NativeToJavaProxyMap (Java=%08x | XPCOM=%08x | IID=%s)\n",
-           (PRUint32) env->CallIntMethod(javaObject, hashCodeMID),
+           (PRUint32) env->CallStaticIntMethod(systemClass, hashCodeMID,
+                                               javaObject),
            (PRUint32) entry, iid_str));
       PR_Free(iid_str);
 #endif
@@ -537,7 +545,7 @@ NativeToJavaProxyMap::Add(JNIEnv* env, nsISupports* aXPCOMObject,
 #ifdef DEBUG_JAVAXPCOM
   char* iid_str = aIID.ToString();
   LOG(("+ NativeToJavaProxyMap (Java=%08x | XPCOM=%08x | IID=%s)\n",
-       (PRUint32) env->CallIntMethod(aProxy, hashCodeMID),
+       (PRUint32) env->CallStaticIntMethod(systemClass, hashCodeMID, aProxy),
        (PRUint32) aXPCOMObject, iid_str));
   PR_Free(iid_str);
 #endif
@@ -572,7 +580,8 @@ NativeToJavaProxyMap::Find(JNIEnv* env, nsISupports* aNativeObject,
 #ifdef DEBUG_JAVAXPCOM
         char* iid_str = aIID.ToString();
         LOG(("< NativeToJavaProxyMap (Java=%08x | XPCOM=%08x | IID=%s)\n",
-             (PRUint32) env->CallIntMethod(*aResult, hashCodeMID),
+             (PRUint32) env->CallStaticIntMethod(systemClass, hashCodeMID,
+                                                 *aResult),
              (PRUint32) aNativeObject, iid_str));
         PR_Free(iid_str);
 #endif
@@ -607,7 +616,8 @@ NativeToJavaProxyMap::Remove(JNIEnv* env, nsISupports* aNativeObject,
 #ifdef DEBUG_JAVAXPCOM
       char* iid_str = aIID.ToString();
       LOG(("- NativeToJavaProxyMap (Java=%08x | XPCOM=%08x | IID=%s)\n",
-           (PRUint32) env->CallIntMethod(item->javaObject, hashCodeMID),
+           (PRUint32) env->CallStaticIntMethod(systemClass, hashCodeMID,
+                                               item->javaObject),
            (PRUint32) aNativeObject, iid_str));
       PR_Free(iid_str);
 #endif
@@ -673,20 +683,20 @@ JavaToXPTCStubMap::Destroy()
 }
 
 nsresult
-JavaToXPTCStubMap::Add(JNIEnv* env, jobject aJavaObject, nsJavaXPTCStub* aProxy)
+JavaToXPTCStubMap::Add(jint aJavaObjectHashCode, nsJavaXPTCStub* aProxy)
 {
   nsAutoLock lock(gJavaXPCOMLock);
 
-  jint hash = env->CallIntMethod(aJavaObject, hashCodeMID);
-  Entry* e = NS_STATIC_CAST(Entry*, PL_DHashTableOperate(mHashTable,
-                                                         NS_INT32_TO_PTR(hash),
-                                                         PL_DHASH_ADD));
+  Entry* e = NS_STATIC_CAST(Entry*,
+                            PL_DHashTableOperate(mHashTable,
+                                           NS_INT32_TO_PTR(aJavaObjectHashCode),
+                                           PL_DHASH_ADD));
   if (!e)
     return NS_ERROR_FAILURE;
 
   NS_ASSERTION(e->key == nsnull,
                "XPTCStub for given Java object already exists in hash table");
-  e->key = hash;
+  e->key = aJavaObjectHashCode;
   e->xptcstub = aProxy;
 
 #ifdef DEBUG_JAVAXPCOM
@@ -705,7 +715,7 @@ JavaToXPTCStubMap::Add(JNIEnv* env, jobject aJavaObject, nsJavaXPTCStub* aProxy)
 }
 
 nsresult
-JavaToXPTCStubMap::Find(JNIEnv* env, jobject aJavaObject, const nsIID& aIID,
+JavaToXPTCStubMap::Find(jint aJavaObjectHashCode, const nsIID& aIID,
                         nsJavaXPTCStub** aResult)
 {
   NS_PRECONDITION(aResult != nsnull, "null ptr");
@@ -715,10 +725,10 @@ JavaToXPTCStubMap::Find(JNIEnv* env, jobject aJavaObject, const nsIID& aIID,
   nsAutoLock lock(gJavaXPCOMLock);
 
   *aResult = nsnull;
-  jint hash = env->CallIntMethod(aJavaObject, hashCodeMID);
-  Entry* e = NS_STATIC_CAST(Entry*, PL_DHashTableOperate(mHashTable,
-                                                         NS_INT32_TO_PTR(hash),
-                                                         PL_DHASH_LOOKUP));
+  Entry* e = NS_STATIC_CAST(Entry*,
+                            PL_DHashTableOperate(mHashTable,
+                                           NS_INT32_TO_PTR(aJavaObjectHashCode),
+                                           PL_DHASH_LOOKUP));
 
   if (PL_DHASH_ENTRY_IS_FREE(e))
     return NS_OK;
@@ -741,13 +751,13 @@ JavaToXPTCStubMap::Find(JNIEnv* env, jobject aJavaObject, const nsIID& aIID,
 }
 
 nsresult
-JavaToXPTCStubMap::Remove(JNIEnv* env, jobject aJavaObject)
+JavaToXPTCStubMap::Remove(jint aJavaObjectHashCode)
 {
-  jint hash = env->CallIntMethod(aJavaObject, hashCodeMID);
-  PL_DHashTableOperate(mHashTable, NS_INT32_TO_PTR(hash), PL_DHASH_REMOVE);
+  PL_DHashTableOperate(mHashTable, NS_INT32_TO_PTR(aJavaObjectHashCode),
+                       PL_DHASH_REMOVE);
 
 #ifdef DEBUG_JAVAXPCOM
-  LOG(("- JavaToXPTCStubMap (Java=%08x)\n", (PRUint32) hash));
+  LOG(("- JavaToXPTCStubMap (Java=%08x)\n", (PRUint32) aJavaObjectHashCode));
 #endif
 
   return NS_OK;
@@ -853,7 +863,8 @@ GetNewOrUsedXPCOMObject(JNIEnv* env, jobject aJavaObject, const nsIID& aIID,
   }
 
   nsJavaXPTCStub* stub;
-  rv = gJavaToXPTCStubMap->Find(env, aJavaObject, aIID, &stub);
+  jint hash = env->CallStaticIntMethod(systemClass, hashCodeMID, aJavaObject);
+  rv = gJavaToXPTCStubMap->Find(hash, aIID, &stub);
   NS_ENSURE_SUCCESS(rv, rv);
   if (stub) {
     // stub is already AddRef'd and QI'd
@@ -881,7 +892,7 @@ GetNewOrUsedXPCOMObject(JNIEnv* env, jobject aJavaObject, const nsIID& aIID,
   if (!stub) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
-  rv = gJavaToXPTCStubMap->Add(env, aJavaObject, stub);
+  rv = gJavaToXPTCStubMap->Add(hash, stub);
   if (NS_FAILED(rv)) {
     delete stub;
     return rv;

extensions/java/xpcom/nsJavaXPCOMBindingUtils.h

@@ -62,6 +62,7 @@
  * Java JNI globals
  *********************/
 
+extern jclass systemClass;
 extern jclass booleanClass;
 extern jclass charClass;
 extern jclass byteClass;
@@ -244,12 +245,12 @@ public:
 
   nsresult Destroy();
 
-  nsresult Add(JNIEnv* env, jobject aJavaObject, nsJavaXPTCStub* aProxy);
+  nsresult Add(jint aJavaObjectHashCode, nsJavaXPTCStub* aProxy);
 
-  nsresult Find(JNIEnv* env, jobject aJavaObject, const nsIID& aIID,
+  nsresult Find(jint aJavaObjectHashCode, const nsIID& aIID,
                 nsJavaXPTCStub** aResult);
 
-  nsresult Remove(JNIEnv* env, jobject aJavaObject);
+  nsresult Remove(jint aJavaObjectHashCode);
 
 protected:
   PLDHashTable* mHashTable;

extensions/java/xpcom/nsJavaXPTCStub.cpp

@@ -56,14 +56,15 @@ nsJavaXPTCStub::nsJavaXPTCStub(jobject aJavaObject, nsIInterfaceInfo *aIInfo)
   jobject weakref = env->NewObject(weakReferenceClass,
                                    weakReferenceConstructorMID, aJavaObject);
   mJavaWeakRef = env->NewGlobalRef(weakref);
+  mJavaRefHashCode = env->CallStaticIntMethod(systemClass, hashCodeMID,
+                                              aJavaObject);
 
 #ifdef DEBUG_JAVAXPCOM
   nsIID* iid;
   mIInfo->GetInterfaceIID(&iid);
   char* iid_str = iid->ToString();
   LOG(("+ nsJavaXPTCStub (Java=%08x | XPCOM=%08x | IID=%s)\n",
-       (PRUint32) env->CallIntMethod(aJavaObject, hashCodeMID),
-       (PRUint32) this, iid_str));
+      (PRUint32) mJavaRefHashCode, (PRUint32) this, iid_str));
   PR_Free(iid_str);
   nsMemory::Free(iid);
 #endif
@@ -121,18 +122,14 @@ nsJavaXPTCStub::ReleaseInternal()
   --mRefCnt;
   NS_LOG_RELEASE(this, mRefCnt, "nsJavaXPTCStub");
   if (mRefCnt == 0) {
+    // delete strong ref; allows Java object to be garbage collected
+    DeleteStrongRef();
+
     // If we have a weak ref, we don't delete this object.
-    JNIEnv* env = GetJNIEnv();
     if (mWeakRefCnt == 0) {
       mRefCnt = 1; /* stabilize */
       Destroy();
-
-      // delete strong ref; allows Java object to be garbage collected
-      env->DeleteGlobalRef(mJavaStrongRef);
       delete this;
-    } else {
-      // delete strong ref; allows Java object to be garbage collected
-      env->DeleteGlobalRef(mJavaStrongRef);
     }
     return 0;
   }
@@ -166,10 +163,8 @@ nsJavaXPTCStub::Destroy()
   nsIID* iid;
   mIInfo->GetInterfaceIID(&iid);
   char* iid_str = iid->ToString();
-  jobject javaObject = env->NewLocalRef(mJavaWeakRef);
   LOG(("- nsJavaXPTCStub (Java=%08x | XPCOM=%08x | IID=%s)\n",
-       (PRUint32) env->CallIntMethod(javaObject, hashCodeMID),
-       (PRUint32) this, iid_str));
+      (PRUint32) mJavaRefHashCode, (PRUint32) this, iid_str));
   PR_Free(iid_str);
   nsMemory::Free(iid);
 #endif
@@ -180,8 +175,11 @@ nsJavaXPTCStub::Destroy()
       delete (nsJavaXPTCStub*) mChildren[i];
     }
 
+    // Since we are destroying this stub, also remove the mapping.
+    // It is possible for mJavaStrongRef to be NULL here.  That is why we
+    // store the hash code value earlier.
     if (gJavaXPCOMInitialized) {
-      gJavaToXPTCStubMap->Remove(env, mJavaStrongRef);
+      gJavaToXPTCStubMap->Remove(mJavaRefHashCode);
     }
   }
 
@@ -211,6 +209,9 @@ nsJavaXPTCStub::ReleaseWeakRef()
 void
 nsJavaXPTCStub::DeleteStrongRef()
 {
+  if (mJavaStrongRef == nsnull)
+    return;
+
   GetJNIEnv()->DeleteGlobalRef(mJavaStrongRef);
   mJavaStrongRef = nsnull;
 }
@@ -1697,8 +1698,7 @@ nsJavaXPTCStub::GetJavaObject()
   mIInfo->GetInterfaceIID(&iid);
   char* iid_str = iid->ToString();
   LOG(("< nsJavaXPTCStub (Java=%08x | XPCOM=%08x | IID=%s)\n",
-       (PRUint32) env->CallIntMethod(javaObject, hashCodeMID),
-       (PRUint32) this, iid_str));
+       (PRUint32) mJavaRefHashCode, (PRUint32) this, iid_str));
   PR_Free(iid_str);
   nsMemory::Free(iid);
 #endif

extensions/java/xpcom/nsJavaXPTCStub.h

@@ -121,6 +121,7 @@ private:
 
   jobject                     mJavaWeakRef;
   jobject                     mJavaStrongRef;
+  jint                        mJavaRefHashCode;
   nsCOMPtr<nsIInterfaceInfo>  mIInfo;
 
   nsVoidArray     mChildren; // weak references (cleared by the children)