Hi Thomas,
I did finally get a chance to look into this, not that I have anything
new to add. We'd need a much more sophisticated scheme to allow the old
"id's" to remain usable but invalid, whilst still allowing the tables to
be reclaimed.
Cheers,
David
On 9/11/2018 5:44 AM, Thomas Stüfe wrote:
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
