JNI - question about jmethodid values.
David Holmes
david.holmes at oracle.com
Wed Nov 28 21:06:15 UTC 2018
Not sure if this discussion is diverging but to be clear there is no way
for a JNI programmer to check whether a jmethodid is valid or not. The
onus is one the programmer to ensure classes are not unloaded while
there are pre-computed jmethodid's. It's a quality of implementation
issue, to me, how/if use of an invalid jmethodid is detected.
Any API that accepts a jmethodid implicitly assumes/expects it is a
valid one. That is not something we should have to state.
David
On 29/11/2018 3:04 am, JC Beyler wrote:
> Sorry I meant:
> https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/design.html#wp17074
>
> :)
> Jc
>
> On Wed, Nov 28, 2018 at 9:02 AM <coleen.phillimore at oracle.com
> <mailto:coleen.phillimore at oracle.com>> wrote:
>
>
>
> On 11/28/18 11:46 AM, JC Beyler wrote:
>> The biggest issue is that the JNI spec has no means of
>> "gracefully" exiting when a jmethodID is NULL, take the call
>> static methods, the spec says:
>>
>> "RETURNS:
>> Returns the result of calling the static Java method.
>>
>> THROWS:
>> Exceptions raised during the execution of the Java method."
>>
>> So now, this has to get fixed to say something like it might throw
>> if the jmethodID is not valid? Seems "messy" to me, I'd rather
>> just fix the spec where it says:
>>
>> PARAMETERS:
>> env: the JNI interface pointer.
>> clazz: a Java class object.
>> methodID: a static method ID.
>>
>> to saying:
>> PARAMETERS:
>> env: the JNI interface pointer.
>> clazz: a Java class object.
>> methodID: a valid static method ID.
>>
>
> I don't think the spec should be changed so that jmethodID prevents
> a class from being unloaded.
>
>> Thanks,
>> Jc
>>
>> On Wed, Nov 28, 2018 at 8:36 AM Thomas Stüfe
>> <thomas.stuefe at gmail.com <mailto:thomas.stuefe at gmail.com>> wrote:
>>
>> What I meant was that since we already pay for the memory leak, we
>> could just change this behavior and handle NULL methodIDs
>> gracefully.
>> We do this already for JVMTI.
>>
>> Otherwise, if we do not what to do this check and consider
>> this to be
>> the caller's responsibility, I do not see the point of keeping the
>> NULLed-out jmethodID tables around. What for, just to make the
>> crash
>> to be a bit more predictable?
>>
>> ..Thomas
>>
>>
>>
>> On Wed, Nov 28, 2018 at 5:22 PM JC Beyler <jcbeyler at google.com
>> <mailto:jcbeyler at google.com>> wrote:
>> >
>> > We pay for it to support easily the JVMTI spec. JNI does not
>> check for NULL and will crash if you pass a jmethod that
>> points to NULL. I've checked that pretty much every method
>> will crash as they all call resolve_jmethod_id at the start.
>> >
>> > As I said before, it's not a bug, the spec is just ambiguous
>> because at the method definitions of the spec it just says the
>> jmethodIDs have to come from a GetMethodID call but the more
>> general spec that both I and Dean quoted say that it is the
>> native agent's job to ensure the class does not get unloaded
>> to keep the jmethod valid [1].
>> >
>> > Thanks,
>> > Jc
>> >
>> > [1]
>> https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/design.html#wp16696
>>
>
> I believe this describes the JNI function table, not jmethodIDs.
>
> No, a jmethodID doesn't keep the class from being unloaded, nor do
> we want it to. This would add undue burden to the GCs, in that they
> would have to trace metadata or some side structure of oops to keep
> classes alive that have jmethodIDs.
>
> Coleen
>
>> >
>> > On Wed, Nov 28, 2018 at 7:38 AM Thomas Stüfe
>> <thomas.stuefe at gmail.com <mailto:thomas.stuefe at gmail.com>> wrote:
>> >>
>> >> On Wed, Nov 28, 2018 at 4:19 PM
>> <coleen.phillimore at oracle.com
>> <mailto:coleen.phillimore at oracle.com>> wrote:
>> >> >
>> >> >
>> >> >
>> >> > On 11/28/18 10:08 AM, Thomas Stüfe wrote:
>> >> > > On Wed, Nov 28, 2018 at 4:03 PM
>> <coleen.phillimore at oracle.com
>> <mailto:coleen.phillimore at oracle.com>> wrote:
>> >> > >>
>> >> > >>
>> >> > >> On 11/28/18 10:00 AM, Thomas Stüfe wrote:
>> >> > >>> On Wed, Nov 28, 2018 at 3:59 PM Thomas Stüfe
>> <thomas.stuefe at gmail.com <mailto:thomas.stuefe at gmail.com>> wrote:
>> >> > >>>> Hi Coleen,
>> >> > >>>>
>> >> > >>>> (moved to svc-dev since David did shoo us off
>> discuss before :-)
>> >> > >>>>
>> >> > >>>> Just to be sure I understand:
>> >> > >>>>
>> >> > >>>>> If the class is unloaded, the jmethodID is
>> cleared. Native code should
>> >> > >>>>> first test whether it's NULL. I think that is the
>> predictable behavior
>> >> > >>>>> that the spec requires.
>> >> > >>>> Wait, really? So, As a JNI caller I should do this:
>> >> > >>>>
>> >> > >>>> jmethodID method;
>> >> > >>>> ..
>> >> > >>>> if (*method == NULL) { .. invalid method id .. } ?
>> >> > >>>>
>> >> > >>>> I thought jmethodid is opaque, and its value itself
>> cannot be changed
>> >> > >>>> from the VM, no?
>> >> > >>>>
>> >> > >>> Oh you probably meant "native code in the VM", not
>> "native JNI code".
>> >> > >>> Sorry for the confusion.
>> >> > >> I did mean native JNI code, but I actually don't know
>> how native JNI
>> >> > >> code is supposed to deal with nulled out jmethodIDs.
>> >> > >>
>> >> > >> Maybe they predictably crash?
>> >> > >>
>> >> > >> Coleen
>> >> > > I always thought it would gracefully reject, e.g. on
>> JVMTI with
>> >> > > JVMTI_ERROR_INVALID_METHODID.
>> >> > >
>> >> > > Save that JC wrote that there are some JNI function
>> sequences where
>> >> > > the VM would still crashes:
>> >> > >
>> >> > > <quote jc>
>> >> > > - Get a jmethodID and remember it
>> >> > > - Wait until the class gets unloaded
>> >> > > - Get the class to get reloaded and try call the
>> old jmethodID
>> >> > > (which now points to NULL), the code will segfault
>> >> > > </quote>
>> >> > >
>> >> > > which looks like just a bug to me.
>> >> >
>> >> > It may be misuse of JNI also. We don't guarantee a lot
>> of things with
>> >> > JNI. Maybe instead of clearing, we could install a
>> Method* that throws
>> >> > NSME.
>> >> >
>> >> > But I guess why leak the jmethodID memory if it's going
>> to crash anyway
>> >> > when using it?
>> >> >
>> >>
>> >> Precisely :) We pay for it, we may just as well use it.
>> >>
>> >> ..Thomas
>> >>
>> >> > Coleen
>> >> >
>> >> > >
>> >> > > ..Thomas
>> >> >
>> >
>> >
>> >
>> > --
>> >
>> > Thanks,
>> > Jc
>>
>>
>>
>> --
>>
>> Thanks,
>> Jc
>
>
>
> --
>
> Thanks,
> Jc
More information about the serviceability-dev
mailing list