Hi JC, On Mon, Nov 26, 2018 at 10:39 PM JC Beyler <jcbey...@google.com> wrote: > > Hi Thomas, > > Not sure I agree either (but this first part of the email might be you and I > rat-holing and we could/should perhaps just let it go :-)): > - The specs do not say at all what happens to the internals of the > jmethodIDs and not even that de-referencing them is safe. You are right that > the spec currently says that the jmethodID should come from a GetMethodID. > But that is all the JNI spec says when it gives a definition of a jmethodID > parameter. However, the JNI general spec about jmethodIDs explicitly does say > [1]: > > "A field or method ID does not prevent the VM from unloading the class from > which the ID has been derived. After the class is unloaded, the method or > field ID becomes invalid. The native code, therefore, must make sure to: > > - keep a live reference to the underlying class, or > - recompute the method or field ID" > > And: "Forcing JNI functions to check for all possible error conditions > degrades the performance of normal (correct) native methods." [1] > > Technically, the JNI spec might want to change the sentences "The methodID > argument must be obtained by calling GetMethodID()" to something like "The > methodID argument must be obtained by calling GetMethodID() and still be > valid, see [1]" > > Turns out that JVMTI spec does provide some of the checks for you and lets > you call with invalid jmethodIds but this is not ok to assume for JNI. > > Btw, when you look at the implementation of the JNI methods right now, if you > try to pass an older jmethodID, it will actually segfault. This might be a > corner case that is a bit awkward but if you do (I have a test example if you > want to see the code to force this path): > - 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
Hmm, would this not be a bug we should fix? Especially since we leak the JNIMethodBlockNodes just to be able to keep their slots alive? > > So remembering jmethodIDs and then assuming that JNI calls will always return > gracefully (whether with an error or not) is actually wrong. For JVMTI, this > seems to hold in terms of spec and implementation from what I see. > > Thanks, > Jc > Thanks, Thomas > [1] > https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/design.html#wp16696 > > On Mon, Nov 26, 2018 at 11:33 AM Thomas Stüfe <thomas.stu...@gmail.com> wrote: >> >> Hey JC, >> >> On Mon, Nov 26, 2018 at 8:15 PM JC Beyler <jcbey...@google.com> wrote: >> > >> > Hi all, >> > >> > Just added my two cents on one comment: >> > >> > On Mon, Nov 26, 2018 at 5:00 AM Thomas Stüfe <thomas.stu...@gmail.com> >> > wrote: >> >> >> >> Hi Peter, >> >> >> >> On Mon, Nov 26, 2018 at 1:02 PM Peter Hull <peterhul...@gmail.com> wrote: >> >> > >> >> > Hi Thomas, >> >> > Thank you very much for the detailed explanation. For your >> >> > information, the relevant NetBeans bug is >> >> > https://issues.apache.org/jira/browse/NETBEANS-1428 >> >> > >> >> > On Sat, Nov 24, 2018 at 3:41 PM Thomas Stüfe <thomas.stu...@gmail.com> >> >> > wrote: >> >> > > A jmethodid is a pointer to malloc'ed memory. >> >> > OK. Just in case I haven't understood it - does this mean a jmethodid, >> >> > once 'created', won't change (after garbage collection or whatever)? >> >> >> >> yes. It lives on, unchanged, forever. Even if the associated class is >> >> unloaded. That must be since outside JNI/JVMTI code may call in with >> >> outdated jmethodids which we must be able to handle gracefully. >> >> >> > >> > This is the current design and most likely will remain for quite a bit but >> > it is not defined by the SPEC to be like this so, in essence, it is an >> > implementation detail that could be not true in future releases ;-) >> >> I politely disagree :) >> >> JNI spec says that you have a function like this: >> >> jmethodID GetMethodID(..) >> >> returning a jmethodid which you can then use in subsequent JNI >> functions. These functions are required to behave in a predictable way >> if the jmethodid has gotten invalid. The spec has no way to inform the >> caller if a jmethodid has gotten invalid by class unloading. Since the >> jmethodid is handed up to the caller and stored by him, there is no >> way to change its value on the fly either. >> >> So even though the spec does not specifically say that jmethodid lives >> forever, it is laid out in a way that prevents jmethodids from ever >> becoming invalid, and from changing their numerical value. This can of >> course change, but not without changing the JNI spec. >> >> I think my point is that Peter can rely on this behavior without fear >> that it will change in the future without him noticing. Before this >> behavior changes, the JNI spec would have to change. >> >> Cheers, Thomas >> >> >> >> >> >> >> >> > >> > Thanks, >> > Jc >> > >> >> >> >> > > >> >> > > But it is not guaranteed to work. I would probably rather use a >> >> > > hashmap or similar. >> >> > I need to look at the implications on more detail but think it would >> >> > make sense to use long/jlong instead of int/jint on all platforms; the >> >> > extra memory use shouldn't be a problem. I think the IDs are just >> >> > stored on the Java side and used to get the method name and signature >> >> > later. That should be a loss-free cast, shouldn't it? >> >> >> >> Sure. >> >> >> >> > > >> >> > > If this is >> >> > > true, this 4x30bit assumption may actually have worked before jdk8, >> >> > > since the java heap is allocated as one continuous space, with the >> >> > > PermGen clustered in one part of it. >> >> > Indeed we did only start to get crashes on JDK9 and later (only >> >> > observed on Windows, macOS seems OK and other platforms have not been >> >> > tested) >> >> > >> >> > Yours sincerely, >> >> > Peter >> >> >> >> Cheers, Thomas >> > >> > >> > >> > -- >> > >> > Thanks, >> > Jc > > > On Mon, Nov 26, 2018 at 1:35 PM David Holmes <david.hol...@oracle.com> wrote: >> >> Please all, don't cc the discuss list once a topic has been directed to >> the correct mailing list! >> >> Thanks, >> David >> >> On 27/11/2018 5:33 am, Thomas Stüfe wrote: >> > Hey JC, >> > >> > On Mon, Nov 26, 2018 at 8:15 PM JC Beyler <jcbey...@google.com> wrote: >> >> >> >> Hi all, >> >> >> >> Just added my two cents on one comment: >> >> >> >> On Mon, Nov 26, 2018 at 5:00 AM Thomas Stüfe <thomas.stu...@gmail.com> >> >> wrote: >> >>> >> >>> Hi Peter, >> >>> >> >>> On Mon, Nov 26, 2018 at 1:02 PM Peter Hull <peterhul...@gmail.com> wrote: >> >>>> >> >>>> Hi Thomas, >> >>>> Thank you very much for the detailed explanation. For your >> >>>> information, the relevant NetBeans bug is >> >>>> https://issues.apache.org/jira/browse/NETBEANS-1428 >> >>>> >> >>>> On Sat, Nov 24, 2018 at 3:41 PM Thomas Stüfe <thomas.stu...@gmail.com> >> >>>> wrote: >> >>>>> A jmethodid is a pointer to malloc'ed memory. >> >>>> OK. Just in case I haven't understood it - does this mean a jmethodid, >> >>>> once 'created', won't change (after garbage collection or whatever)? >> >>> >> >>> yes. It lives on, unchanged, forever. Even if the associated class is >> >>> unloaded. That must be since outside JNI/JVMTI code may call in with >> >>> outdated jmethodids which we must be able to handle gracefully. >> >>> >> >> >> >> This is the current design and most likely will remain for quite a bit >> >> but it is not defined by the SPEC to be like this so, in essence, it is >> >> an implementation detail that could be not true in future releases ;-) >> > >> > I politely disagree :) >> > >> > JNI spec says that you have a function like this: >> > >> > jmethodID GetMethodID(..) >> > >> > returning a jmethodid which you can then use in subsequent JNI >> > functions. These functions are required to behave in a predictable way >> > if the jmethodid has gotten invalid. The spec has no way to inform the >> > caller if a jmethodid has gotten invalid by class unloading. Since the >> > jmethodid is handed up to the caller and stored by him, there is no >> > way to change its value on the fly either. >> > >> > So even though the spec does not specifically say that jmethodid lives >> > forever, it is laid out in a way that prevents jmethodids from ever >> > becoming invalid, and from changing their numerical value. This can of >> > course change, but not without changing the JNI spec. >> > >> > I think my point is that Peter can rely on this behavior without fear >> > that it will change in the future without him noticing. Before this >> > behavior changes, the JNI spec would have to change. >> > >> > Cheers, Thomas >> > >> > >> > >> > >> > >> > >> > >> >> >> >> Thanks, >> >> Jc >> >> >> >>> >> >>>>> >> >>>>> But it is not guaranteed to work. I would probably rather use a >> >>>>> hashmap or similar. >> >>>> I need to look at the implications on more detail but think it would >> >>>> make sense to use long/jlong instead of int/jint on all platforms; the >> >>>> extra memory use shouldn't be a problem. I think the IDs are just >> >>>> stored on the Java side and used to get the method name and signature >> >>>> later. That should be a loss-free cast, shouldn't it? >> >>> >> >>> Sure. >> >>> >> >>>>> >> >>>>> If this is >> >>>>> true, this 4x30bit assumption may actually have worked before jdk8, >> >>>>> since the java heap is allocated as one continuous space, with the >> >>>>> PermGen clustered in one part of it. >> >>>> Indeed we did only start to get crashes on JDK9 and later (only >> >>>> observed on Windows, macOS seems OK and other platforms have not been >> >>>> tested) >> >>>> >> >>>> Yours sincerely, >> >>>> Peter >> >>> >> >>> Cheers, Thomas >> >> >> >> >> >> >> >> -- >> >> >> >> Thanks, >> >> Jc > > > > -- > > Thanks, > Jc
