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.phillim...@oracle.com <mailto:coleen.phillim...@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.stu...@gmail.com <mailto:thomas.stu...@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 <jcbey...@google.com
        <mailto:jcbey...@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.stu...@gmail.com <mailto:thomas.stu...@gmail.com>> wrote:
        >>
        >> On Wed, Nov 28, 2018 at 4:19 PM
        <coleen.phillim...@oracle.com
        <mailto:coleen.phillim...@oracle.com>> wrote:
        >> >
        >> >
        >> >
        >> > On 11/28/18 10:08 AM, Thomas Stüfe wrote:
        >> > > On Wed, Nov 28, 2018 at 4:03 PM
        <coleen.phillim...@oracle.com
        <mailto:coleen.phillim...@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.stu...@gmail.com <mailto:thomas.stu...@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

Reply via email to