JDK-8229900 : RedefineDeleteJmethod.java fails with -Xcheck:jni
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 14
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2019-08-20
  • Updated: 2019-10-10
  • Resolved: 2019-10-07
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 14
14 b18Fixed
Related Reports
Relates :  
JDK-8181171 - Deleting method for RedefineClasses breaks ResolvedMethodName
Relates :  
JDK-8232120 - com/sun/jdi/InvokeTest fails with -Xcheck:jni: assert(k->is_instance_klass()) failed: cast to InstanceKlass
Description
Running serviceability/jvmti/RedefineClasses/RedefineDeleteJmethod.java with -Xcheck:jni fails with the following error:

FATAL ERROR in native method: non-weak methodID passed to JNI call
	at RedefineDeleteJmethod.jniCallDeleteMe(Native Method)
	at RedefineDeleteJmethod.test(RedefineDeleteJmethod.java:72)
	at RedefineDeleteJmethod.main(RedefineDeleteJmethod.java:90)
	at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(java.base@14-internal/Native Method)
	at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(java.base@14-internal/NativeMethodAccessorImpl.java:62)
	at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(java.base@14-internal/DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(java.base@14-internal/Method.java:565)
	at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
	at java.lang.Thread.run(java.base@14-internal/Thread.java:830)
Comments
URL: https://hg.openjdk.java.net/jdk/jdk/rev/35ce0ad5870a User: coleenp Date: 2019-10-07 14:04:17 +0000
07-10-2019
We don't check: resolve_method_id(mid->method_holder()) == clazz because the JNI lookup can find a class in a superclass or interface, so this check isn't straightforward. But we can test is_subtype_of(mid->method_holder()).
30-09-2019
The purpose of this RedefineDeleteJMethod.java was to call Unsafe.throwNoSuchMethodError() for the case where a jmethodID points to a deleted method. This was to make this undefined behavior have a better outcome. As with all cases of adding and deleting methods, this isn't straightforward, because the Method's class is Unsafe instead of the class that owns the method that was deleted. Also, we generally do not correct invalid JNI code, so we shouldn't for this case either. I'm going to remove this test and the special case in redefinition.
30-09-2019
I think this bug should filed under serviceability to have the test fixed. We could have a separate runtime bug to fix the JNI check error message and potentially enhance the check further.
21-08-2019
ILW = MLM = P4
20-08-2019
Actually it's probably a bit more involved than that - the checking isn't really making a lot of sense to me. We create a jMethodId for a given method of a given class, and the jMethodId embodies the Method* for the actual method. We then attempt to make a JNI call on a given class using that jMethodId: (*env)->CallStaticIntMethod(env, cls, mid); but the validation is done independent of the "cls" involved in the call! So what we execute is: Method* jniCheck::validate_jmethod_id(JavaThread* thr, jmethodID method_id) { ASSERT_OOPS_ALLOWED; // do the fast jmethodID check first Method* moop = Method::checked_resolve_jmethod_id(method_id); if (moop == NULL) { ReportJNIFatalError(thr, fatal_wrong_class_or_method); } // jmethodIDs are supposed to be weak handles in the class loader data, // but that can be expensive so check it last else if (!Method::is_method_id(method_id)) { ReportJNIFatalError(thr, fatal_non_weak_method); } return moop; } which says nothing about the use of that jMethodId with the specific class. So the check seems incomplete to me. Where we have: void jniCheck::validate_call_class(JavaThread* thr, jclass clazz, jmethodID method_id) { /* validate the class being passed */ ASSERT_OOPS_ALLOWED; jniCheck::validate_jmethod_id(thr, method_id); jniCheck::validate_class(thr, clazz, false); } we should also have a check of the form: resolve_method_id(mid->method_holder()) == clazz In any case we are detecting in this test that the cached jMethodId is no longer valid - though exactly what part of the test is failing is hard to say by code inspection.
20-08-2019
I find the error message here to be very confusing. What is actually happening is that a validity check for a jMethodID is failing: // jmethodIDs are supposed to be weak handles in the class loader data, // but that can be expensive so check it last else if (!Method::is_method_id(method_id)) { ReportJNIFatalError(thr, fatal_non_weak_method); } where: bool Method::is_method_id(jmethodID mid) { Method* m = resolve_jmethod_id(mid); assert(m != NULL, "should be called with non-null method"); InstanceKlass* ik = m->method_holder(); ClassLoaderData* cld = ik->class_loader_data(); if (cld->jmethod_ids() == NULL) return false; return (cld->jmethod_ids()->contains((Method**)mid)); } so "non_weak_method" seems a completely inappropriate description to me. That aside the test is flawed in that it creates the jMethodID on first use and then reuses it on subsequent calls. That would be okay if always used with the same class but this test redefines the class, so the cached jMethodId is not valid for use with the redefined class - which I think is what -Xcheck:jni is detecting.
20-08-2019
> non-weak methodID passed to JNI call Why should we be passing a weak methodID ?? For that matter what _is_ a "weak" methodID ??
20-08-2019
RedefineDeleteJmethod was added by JKD-8181171, which was more recent (2/2019) than JDK-8189766 (5/2018) where running tier1/2 tests with -Xcheck:jni was known to pass.
20-08-2019