[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: Clarifying jmethodID Usage and Potential JVM Crashes
From: Jaroslav_Bachorík <jaroslav.bachorik () datadoghq ! com>
Date: 2023-05-31 13:23:43
Message-ID: CAPb8aJ53f-euLF1_1AE_4ok-8Hr3wBnrGpu7Yi1=N5RSJbzN-g () mail ! gmail ! com
[Download RAW message or body]
On Wed, May 31, 2023 at 3:16 PM Dan Heidinga <heidinga@redhat.com> wrote:
> On Wed, May 31, 2023 at 7:13 AM Jaroslav Bachorík <
> jaroslav.bachorik@datadoghq.com> wrote:
>
>> Dear Team,
>>
>> I've been investigating the unusual JVM crashes occurring in JVMTI calls
>> on a J9 JVM. During my investigation, I scrutinized the `jmethodID`
>> definition closely, available here: [jmethodID definition](
>> https://docs.oracle.com/en/java/javase/17/docs/specs/jvmti.html#jmethodID
>> ).
>>
>> To paraphrase, the definition suggests that `jmethodID` identifies a Java
>> method, initializer, or constructor. These identifiers, once returned by
>> JVM TI functions and events, can be safely stored. However, when the class
>> is unloaded, they become invalid, rendering them unsuitable for use.
>>
>
> The "typical" usage pattern is to create a JNI reference (local or global)
> to the relevant class and keep that reference for the duration of the
> jmethodID's life. If the jmethodID needs to exist past the end of the
> local method, then it needs to be accompanied in some way by a global
> reference to the class. This works great for JNI as you typically need to
> look up the class first before looking up methods. JVMTI is more a
> challenge here as it's possible to get the jmethodID without already
> holding the class - GetMethodDeclaringClass can help here if the jmethodID
> is still valid.
>
Unfortunately, GetMethodDeclaringClass would crash if the jmethodID is not
valid any more, so it really can not be used safely as well.
>
>
>> My interpretation is that the JVMTI user should verify the validity of a
>> `jmethodID` value before using it to prevent potential crashes. Would you
>> agree with this interpretation?
>>
>
> Mostly. I agree that using an invalid jmethodID will cause crashes but
> unfortunately, I'm not aware of any spec'd methods to verify its validity.
>
>
>>
>> This sounds like a sensible requirement, but its practical application
>> remains unclear. As far as I know, methods can be unloaded concurrently to
>> the native code executing JVMTI functions. This introduces a potential race
>> condition where the JVM unloads the methods during the check->use flow,
>> making it only a partial solution. To complicate matters further, no method
>> exists to confirm whether a `jmethodID` is valid.
>>
>> Theoretically, we could monitor the `CompiledMethodUnload` event to track
>> the validity state, creating a constantly expanding set of unloaded
>> `jmethodID` values or a bloom filter, if one does not care about few
>> potential false positives. This strategy, however, doesn't address the
>> potential race condition, and it could even exacerbate it due to possible
>> event delays. This delay might mistakenly validate a `jmethodID` value that
>> has already been unloaded, but for which the event hasn't been delivered
>> yet.
>>
>
> And CompileMethodUnloaded isn't the right event either as the jmethodID
> could still be valid. If there was a ClassUnloaded event, and a mapping
> from class->jmethodID, it would be possible to invalidate the jmethodIDs
> but I'm not aware of any spec'd events that provide those details.
>
Yes, I see that there are more pieces missing here :(
>
>
>>
>> Honestly, I don't see a way to use `jmethodID` safely unless the code
>> using it suspends the entire JVM and doesn't resume until it's finished
>> with that `jmethodID`. Any other approach might lead to JVM crashes, as
>> we've observed with J9.
>>
>> Lastly, it's noteworthy that Hotspot takes meticulous measures to ensure
>> that using jmethodIDs for unloaded methods doesn't crash the JVM and even
>> provides useful information. This observation has led me to question
>> whether the documentation aligns with the Hotspot implementation,
>> especially given that following closely the documentation appears to
>> increase the risk associated with the use of `jmethodID` values.
>>
>
> I did a pass through the spec after becoming aware of the crashes you were
> seeing on J9 and couldn't find a spec-compliant way to prevent the
> crashes. It feels like there's some missing methods in the spec to make
> your use case safe.
>
Indeed. This is all even worse when one would use ASGCT as there is not
even a theoretical possibility to create a strong reference to a class
holding a particular jmethodID - doing so for every single collected frame
would be a performance nightmare, not even mentioning the fact that ASGCT
is used from a signal handler where any complex JVMTI operations are simply
not safe.
-JB-
>
> --Dan
>
>
>> I welcome your thoughts and perspectives on this matter.
>>
>> Best regards,
>>
>> Jaroslav
>>
>
[Attachment #3 (text/html)]
<div dir="ltr"><div dir="ltr">On Wed, May 31, 2023 at 3:16 PM Dan Heidinga <<a \
href="mailto:heidinga@redhat.com">heidinga@redhat.com</a>> wrote:<br></div><div \
class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div \
dir="ltr">On Wed, May 31, 2023 at 7:13 AM Jaroslav Bachorík <<a \
href="mailto:jaroslav.bachorik@datadoghq.com" \
target="_blank">jaroslav.bachorik@datadoghq.com</a>> wrote:<br></div><div \
class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Dear \
Team,<br><br>I've been investigating the unusual JVM crashes occurring in JVMTI \
calls on a J9 JVM. During my investigation, I scrutinized the `jmethodID` definition \
closely, available here: [jmethodID definition](<a \
href="https://docs.oracle.com/en/java/javase/17/docs/specs/jvmti.html#jmethodID" \
target="_blank">https://docs.oracle.com/en/java/javase/17/docs/specs/jvmti.html#jmethodID</a>).<br><br>To \
paraphrase, the definition suggests that `jmethodID` identifies a Java method, \
initializer, or constructor. These identifiers, once returned by JVM TI functions and \
events, can be safely stored. However, when the class is unloaded, they become \
invalid, rendering them unsuitable for \
use.<br></div></blockquote><div><br></div><div>The "typical" usage pattern \
is to create a JNI reference (local or global) to the relevant class and keep that \
reference for the duration of the jmethodID's life. If the jmethodID needs to \
exist past the end of the local method, then it needs to be accompanied in some way \
by a global reference to the class. This works great for JNI as you typically need \
to look up the class first before looking up methods. JVMTI is more a challenge \
here as it's possible to get the jmethodID without already holding the class - \
GetMethodDeclaringClass can help here if the jmethodID is still \
valid.</div></div></div></blockquote><div><br></div><div>Unfortunately, \
GetMethodDeclaringClass would crash if the jmethodID is not valid any more, so it \
really can not be used safely as well.</div><div> </div><blockquote \
class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div \
class="gmail_quote"><div><br></div><blockquote class="gmail_quote" style="margin:0px \
0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div \
dir="ltr"><br>My interpretation is that the JVMTI user should verify the validity of \
a `jmethodID` value before using it to prevent potential crashes. Would you agree \
with this interpretation?<br></div></blockquote><div><br></div><div>Mostly. I agree \
that using an invalid jmethodID will cause crashes but unfortunately, I'm not \
aware of any spec'd methods to verify its validity.</div><div> </div><blockquote \
class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br>This sounds like a sensible \
requirement, but its practical application remains unclear. As far as I know, methods \
can be unloaded concurrently to the native code executing JVMTI functions. This \
introduces a potential race condition where the JVM unloads the methods during the \
check->use flow, making it only a partial solution. To complicate matters further, \
no method exists to confirm whether a `jmethodID` is valid.<br><br>Theoretically, we \
could monitor the `CompiledMethodUnload` event to track the validity state, creating \
a constantly expanding set of unloaded `jmethodID` values or a bloom filter, if one \
does not care about few potential false positives. This strategy, however, \
doesn't address the potential race condition, and it could even exacerbate it due \
to possible event delays. This delay might mistakenly validate a `jmethodID` value \
that has already been unloaded, but for which the event hasn't been delivered \
yet.<br></div></blockquote><div><br></div><div>And CompileMethodUnloaded isn't \
the right event either as the jmethodID could still be valid. If there was a \
ClassUnloaded event, and a mapping from class->jmethodID, it would be possible to \
invalidate the jmethodIDs but I'm not aware of any spec'd events that provide \
those details.</div></div></div></blockquote><div><br></div><div>Yes, I see that \
there are more pieces missing here :(</div><div> </div><blockquote \
class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> \
</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px \
solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br>Honestly, I don't see \
a way to use `jmethodID` safely unless the code using it suspends the entire JVM and \
doesn't resume until it's finished with that `jmethodID`. Any other approach \
might lead to JVM crashes, as we've observed with J9.<br><br>Lastly, it's \
noteworthy that Hotspot takes meticulous measures to ensure that using jmethodIDs for \
unloaded methods doesn't crash the JVM and even provides useful information. This \
observation has led me to question whether the documentation aligns with the Hotspot \
implementation, especially given that following closely the documentation appears to \
increase the risk associated with the use of `jmethodID` \
values.<br></div></blockquote><div><br></div><div>I did a pass through the spec after \
becoming aware of the crashes you were seeing on J9 and couldn't find a \
spec-compliant way to prevent the crashes. It feels like there's some missing \
methods in the spec to make your use case \
safe.</div></div></div></blockquote><div><br></div><div>Indeed. This is all even \
worse when one would use ASGCT as there is not even a theoretical possibility to \
create a strong reference to a class holding a particular jmethodID - doing so for \
every single collected frame would be a performance nightmare, not even mentioning \
the fact that ASGCT is used from a signal handler where any complex JVMTI operations \
are simply not safe.</div><div><br></div><div>-JB-</div><div> </div><blockquote \
class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div \
class="gmail_quote"><div><br></div><div>--Dan</div><div><br></div><blockquote \
class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid \
rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br>I welcome your thoughts and \
perspectives on this matter.<br><br>Best regards, <br><br>Jaroslav</div> \
</blockquote></div></div> </blockquote></div></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic