Hi JC, On Thu, Nov 8, 2018 at 5:49 PM JC Beyler <jcbey...@google.com> wrote: > > Hi Thomas, > > When I follow the code flow, yes that seems right; your comment " a caller > may hand it in long after the class has been unloaded, and this should be > handled gracefully" is exactly what the spec says for example for a lot of > the JVMTI methods that take a methodId such as SetBreakpoint[1] for example.
Oh right. I was struggling to find a good example for why we cannot let jmethodids expire. But sure, the debugger could have set there for an hour after calling GetClassMethods, and the jmethodids could be all stale. > > Doing it this way is the easiest (safest?) way to ensure that if a class is > unloaded, there is a means to be passing invalid method ids to JVMTI methods > without having everything break or having to have in memory what methods are > still valid. Its also quite cheap. We pay 8 bytes of leaked memory per jmethodid, plus a bit of overhead. I struggle to come up with a better scheme. I guess one could, with some sort of offset based addressing, trim this down to 32bit? Or some magic with un-committing the underlying memory under the JNIMethodBlockNodes but keeping the address space reserved, and then catching the segfault with SafeFetch. But I do not know if it would be worth the effort. > [1] > https://docs.oracle.com/javase/8/docs/platform/jvmti/jvmti.html#SetBreakpoint > > Or more in detail, a lot of JVMTI methods have a nice: > NULL_CHECK(method_oop, JVMTI_ERROR_INVALID_METHODID); > > That allows to do this check and, so technically, the comment could be > amended to say "JVMTI also is expecting this". > I agree. > Is there a real case where this leak is becoming noticeable? > Yes, we have this client which uses our VM in weird and intricate ways. Embedded in an own launcher, lots of JNI coding. They seem to be doing just that: over and over reloading classes in new class loaders and accessing them via JNI. Last I saw they allocated 250 MB worth of JNIMethodBlockNodes, which would amount to roughly 3 million jmethodid handles... > Thanks, > Jc > Thanks for looking! Best Regards, Thomas > > > On Thu, Nov 8, 2018 at 12:02 AM Thomas Stüfe <thomas.stu...@gmail.com> wrote: >> >> .. moving to serviceability - maybe you can help me. >> >> Thanks! Thomas >> >> >> ---------- Forwarded message --------- >> From: Thomas Stüfe <thomas.stu...@gmail.com> >> Date: Wed, Nov 7, 2018, 11:38 >> Subject: JNIMethodBlockNode leak question >> To: Hotspot dev runtime <hotspot-runtime-...@openjdk.java.net> >> >> >> Hi all, >> >> I wonder whether I understand this correctly: >> >> When a caller requests a jmethodId bei either calling >> jni-GetMethodId() or jvmti-GetClassMethods(), the returned jmethodId >> is a pointer into a slot of a table containing the respective Method* >> pointers. That table is tied to the ClassLoaderData. When the >> classloader is unloaded and its ClassLoaderData deleted, we do not >> delete that table, but rather deliberately leak it after zeroing it >> out. The reason - if I understand the comment in ~ClassLoaderData >> correctly - is that jmethodId live forever - a caller may hand it in >> long after the class has been unloaded, and this should be handled >> gracefully. >> >> We cannot simply delete its JNIMethodBlockNode, since the jmethodId >> then would either point to unmapped memory - which we could handle >> with SafeFetch - but worse could point to memory reused for a >> different purpose. >> >> We also could not reuse that slot, since then that jmethodId would >> point to a different method? Apart from the fact that we would need >> infrastructure for that, a global pool of JNIMethodBlockNode, with >> associated locking etc. >> >> Have I understood this correctly or am I off somewhere? >> >> And if I am right, that would mean that were we to generate classes >> over and over again in new class loaders and accessing their methods >> with JNI, we would have a slowly growing leak, even if we clean up the >> loaders? >> >> Thanks, Thomas > > > > -- > > Thanks, > Jc
