Bug in JNI RegisterNatives?

Mon Oct 9 23:46:00 GMT 2006

>>>>> "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);
