JNI - question about jmethodid values.

[JC Beyler]

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.

Thanks,
Jc

On Wed, Nov 28, 2018 at 8:36 AM Thomas Stüfe <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> 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
> >
> > On Wed, Nov 28, 2018 at 7:38 AM Thomas Stüfe <thomas.stuefe at gmail.com>
> wrote:
> >>
> >> On Wed, Nov 28, 2018 at 4:19 PM <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>
> 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> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20181128/b2cd8db3/attachment.html>