Re: Bug in JNI RegisterNatives?

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Juerg" == Juerg Lehni <juerg@scratchdisk.com> writes:

Juerg> I am wondering wether this patch will eventally be included
Juerg> I've submitted it more than 3 months ago, and nothing seems to have
Juerg> happened since.

Juerg> Is there anything I need to do to get it accepted?

I think your patch is too big to go in without an assignment.  And,
given the proximity of 4.2, and the time needed to get the paperwork
in order, I thought I would just rewrite it.

Basically I just added a new class name field to the entries in the
hash table.  Could you try this patch with your test case and let me
know if it works?  If it does I will check it in.

Thanks, and sorry for the difficulty here.

Tom

Index: ChangeLog
from  Tom Tromey  <tromey@redhat.com>
	Bryce McKinlay  <mckinlay@redhat.com>

	* java/lang/natClass.cc (_Jv_GetClassNameUtf8): New function.
	* java/lang/Class.h (_Jv_GetClassNameUtf8): Declare.
	* jni.cc (struct NativeMethodCacheEntry): New struct.
	(nathash): Changed type.
	(hash): Updated.
	(nathash_find_slot): Likewise.
	(nathash_find): Likewise.
	(natrehash): Likewise.
	(nathash_add): Likewise.
	(_Jv_JNI_RegisterNatives): Likewise.
	(_Jv_LookupJNIMethod): Likewise.
	Idea from Juerg Lehni <juerg@scratchdisk.com>

Index: jni.cc
===================================================================
--- jni.cc	(revision 117258)
+++ jni.cc	(working copy)
@@ -1789,8 +1789,13 @@
 
 
 
+struct NativeMethodCacheEntry : public JNINativeMethod
+{
+  char *className;
+};
+
 // Hash table of native methods.
-static JNINativeMethod *nathash;
+static NativeMethodCacheEntry *nathash;
 // Number of slots used.
 static int nathash_count = 0;
 // Number of slots available.  Must be power of 2.
@@ -1800,11 +1805,15 @@
 
 // Compute a hash value for a native method descriptor.
 static int
-hash (const JNINativeMethod *method)
+hash (const NativeMethodCacheEntry *method)
 {
   char *ptr;
   int hash = 0;
 
+  ptr = method->className;
+  while (*ptr)
+    hash = (31 * hash) + *ptr++;
+
   ptr = method->name;
   while (*ptr)
     hash = (31 * hash) + *ptr++;
@@ -1817,8 +1826,8 @@
 }
 
 // Find the slot where a native method goes.
-static JNINativeMethod *
-nathash_find_slot (const JNINativeMethod *method)
+static NativeMethodCacheEntry *
+nathash_find_slot (const NativeMethodCacheEntry *method)
 {
   jint h = hash (method);
   int step = (h ^ (h >> 16)) | 1;
@@ -1827,7 +1836,7 @@
 
   for (;;)
     {
-      JNINativeMethod *slotp = &nathash[w];
+      NativeMethodCacheEntry *slotp = &nathash[w];
       if (slotp->name == NULL)
 	{
 	  if (del >= 0)
@@ -1838,7 +1847,8 @@
       else if (slotp->name == DELETED_ENTRY)
 	del = w;
       else if (! strcmp (slotp->name, method->name)
-	       && ! strcmp (slotp->signature, method->signature))
+	       && ! strcmp (slotp->signature, method->signature)
+	       && ! strcmp (slotp->className, method->className))
 	return slotp;
       w = (w + step) & (nathash_size - 1);
     }
@@ -1846,11 +1856,11 @@
 
 // Find a method.  Return NULL if it isn't in the hash table.
 static void *
-nathash_find (JNINativeMethod *method)
+nathash_find (NativeMethodCacheEntry *method)
 {
   if (nathash == NULL)
     return NULL;
-  JNINativeMethod *slot = nathash_find_slot (method);
+  NativeMethodCacheEntry *slot = nathash_find_slot (method);
   if (slot->name == NULL || slot->name == DELETED_ENTRY)
     return NULL;
   return slot->fnPtr;
@@ -1863,23 +1873,23 @@
     {
       nathash_size = 1024;
       nathash =
-	(JNINativeMethod *) _Jv_AllocBytes (nathash_size
-					    * sizeof (JNINativeMethod));
+	(NativeMethodCacheEntry *) _Jv_AllocBytes (nathash_size
+						   * sizeof (NativeMethodCacheEntry));
     }
   else
     {
       int savesize = nathash_size;
-      JNINativeMethod *savehash = nathash;
+      NativeMethodCacheEntry *savehash = nathash;
       nathash_size *= 2;
       nathash =
-	(JNINativeMethod *) _Jv_AllocBytes (nathash_size
-					    * sizeof (JNINativeMethod));
+	(NativeMethodCacheEntry *) _Jv_AllocBytes (nathash_size
+						   * sizeof (NativeMethodCacheEntry));
 
       for (int i = 0; i < savesize; ++i)
 	{
 	  if (savehash[i].name != NULL && savehash[i].name != DELETED_ENTRY)
 	    {
-	      JNINativeMethod *slot = nathash_find_slot (&savehash[i]);
+	      NativeMethodCacheEntry *slot = nathash_find_slot (&savehash[i]);
 	      *slot = savehash[i];
 	    }
 	}
@@ -1887,16 +1897,17 @@
 }
 
 static void
-nathash_add (const JNINativeMethod *method)
+nathash_add (const NativeMethodCacheEntry *method)
 {
   if (3 * nathash_count >= 2 * nathash_size)
     natrehash ();
-  JNINativeMethod *slot = nathash_find_slot (method);
+  NativeMethodCacheEntry *slot = nathash_find_slot (method);
   // If the slot has a real entry in it, then there is no work to do.
   if (slot->name != NULL && slot->name != DELETED_ENTRY)
     return;
-  // FIXME
+  // FIXME: memory leak?
   slot->name = strdup (method->name);
+  slot->className = strdup (method->className);
   // This was already strduped in _Jv_JNI_RegisterNatives.
   slot->signature = method->signature;
   slot->fnPtr = method->fnPtr;
@@ -1912,7 +1923,7 @@
   // the nathash table.
   JvSynchronize sync (global_ref_table);
 
-  JNINativeMethod dottedMethod;
+  NativeMethodCacheEntry dottedMethod;
 
   // Look at each descriptor given us, and find the corresponding
   // method in the class.
@@ -1928,8 +1939,11 @@
 	  // Copy this JNINativeMethod and do a slash to dot
 	  // conversion on the signature.
 	  dottedMethod.name = methods[j].name;
+	  // FIXME: we leak a little memory here if the method
+	  // is not found.
 	  dottedMethod.signature = strdup (methods[j].signature);
 	  dottedMethod.fnPtr = methods[j].fnPtr;
+	  dottedMethod.className = _Jv_GetClassNameUtf8 (klass)->chars();
 	  char *c = dottedMethod.signature;
 	  while (*c)
 	    {
@@ -2172,9 +2186,10 @@
   buf[name_length] = '\0';
   strncpy (buf + name_length + 1, signature->chars (), sig_length);
   buf[name_length + sig_length + 1] = '\0';
-  JNINativeMethod meth;
+  NativeMethodCacheEntry meth;
   meth.name = buf;
   meth.signature = buf + name_length + 1;
+  meth.className = _Jv_GetClassNameUtf8(klass)->chars();
   function = nathash_find (&meth);
   if (function != NULL)
     return function;
Index: java/lang/natClass.cc
===================================================================
--- java/lang/natClass.cc	(revision 117258)
+++ java/lang/natClass.cc	(working copy)
@@ -1259,3 +1259,11 @@
   return NULL;
 }
 #endif
+
+// Return Utf8 name of a class. This function is here for code that
+// can't access klass->name directly.
+_Jv_Utf8Const*
+_Jv_GetClassNameUtf8 (jclass klass)
+{
+  return klass->name;
+}
Index: java/lang/Class.h
===================================================================
--- java/lang/Class.h	(revision 117258)
+++ java/lang/Class.h	(working copy)
@@ -231,6 +231,7 @@
 jmethodID _Jv_FromReflectedConstructor (java::lang::reflect::Constructor *);
 jint JvNumMethods (jclass);
 jmethodID JvGetFirstMethod (jclass);
+_Jv_Utf8Const *_Jv_GetClassNameUtf8 (jclass);
 
 #ifdef INTERPRETER
 // Finds a desired interpreter method in the given class or NULL if not found
@@ -474,6 +475,7 @@
   friend jmethodID (::_Jv_FromReflectedConstructor) (java::lang::reflect::Constructor *);
   friend jint (::JvNumMethods) (jclass);
   friend jmethodID (::JvGetFirstMethod) (jclass);
+  friend _Jv_Utf8Const *::_Jv_GetClassNameUtf8 (jclass);
 #ifdef INTERPRETER
   friend _Jv_MethodBase *(::_Jv_FindInterpreterMethod) (jclass klass,
 							jmethodID desired_method);