On Wed, May 31, 2023 at 7:13 AM Jaroslav Bachorík < jaroslav.bacho...@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. > 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. > > 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. --Dan > I welcome your thoughts and perspectives on this matter. > > Best regards, > > Jaroslav >
