[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: JNI - question about jmethodid values.
From:       David Holmes <david.holmes () oracle ! com>
Date:       2018-11-28 21:06:15
Message-ID: 3ec7b62e-97af-0939-28c6-6e9b756644b4 () oracle ! com
[Download RAW message or body]

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@oracle.com 
> <mailto:coleen.phillimore@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@gmail.com <mailto:thomas.stuefe@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@google.com
>>         <mailto:jcbeyler@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@gmail.com <mailto:thomas.stuefe@gmail.com>> wrote:
>>         >>
>>         >> On Wed, Nov 28, 2018 at 4:19 PM
>>         <coleen.phillimore@oracle.com
>>         <mailto:coleen.phillimore@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@oracle.com
>>         <mailto:coleen.phillimore@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@gmail.com <mailto:thomas.stuefe@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
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic