RFR: 8313816: Accessing jmethodID might lead to spurious crashes

[Andrei Pangin]

On Mon, 7 Aug 2023 00:50:30 GMT, Jaroslav Bachorik <jbachorik at openjdk.org> wrote:

> This is a best effort attempt to harmonize the handling of jmethodIDs when the instanceKlass internal structures are being deallocated (eg. due to `ClassLoaderData::free_deallocate_list`).
> In all other instances, the jmethodID is NULLed (or set to `_free_method`) before the references Method structures are deallocated. This actually makes possible to query such jmethodIDs without crashing JVM even if the corresponding classes/methods were unloaded.
> 
> While it is understandable why JVM can not keep the method metadata around forever it really should be possible to at least assert the validity of a jmethodID without crashing JVM. If nothing else, this makes using the JVMTI functions `GetAllStackTraces` or `GetStackTrace` a risk to use in JVMTI agents - the jmethodids can become invalid the very next moment after the stacktrace is obtained.

Changes requested by apangin (no project role).

src/hotspot/share/oops/instanceKlass.cpp line 2834:

> 2832:     for (size_t i = 1; i <= count; i++) {
> 2833:       *((Method**)jmeths[i]) = nullptr;
> 2834:     }

The proposed change looks suspicious: the structure being zeroed is released on the very next line.
So, this change does not seem to fix anything - it only makes a bug (if any) less obvious.
I would rather do the opposite: poison the memory to make sure any attempt to access freed memory will immediately fail and thus reveal the bug in the code.

P.S. JVM TI functions do not access this structure; they use jmethodID cache in ClassLoaderData which is cleared but never released, see https://github.com/openjdk/jdk/blob/87b08b6e0192d88025c2275c7dd2c4bdecda58e8/src/hotspot/share/classfile/classLoaderData.cpp#L609C14-L622

-------------

PR Review: https://git.openjdk.org/jdk/pull/15171#pullrequestreview-1566354864
PR Review Comment: https://git.openjdk.org/jdk/pull/15171#discussion_r1286518429