RFR: 8313816: Accessing jmethodID might lead to spurious crashes

[Jaroslav Bachorik]

On Wed, 15 Nov 2023 18:47:19 GMT, Jaroslav Bachorik <jbachorik at openjdk.org> wrote:

>> src/hotspot/share/oops/instanceKlass.cpp line 531:
>> 
>>> 529: 
>>> 530: void InstanceKlass::deallocate_methods(ClassLoaderData* loader_data,
>>> 531:                                        Array<Method*>* methods, InstanceKlass* klass) {
>> 
>> An explicit boolean parameter would be cleaner/clearer.
>
> I just removed the `klass` argument. It is not really used anyway.

I actually ended up with a boolean parameter here.

>> src/hotspot/share/oops/method.hpp line 730:
>> 
>>> 728:   // so handles are not used to avoid deadlock.
>>> 729:   jmethodID find_jmethod_id_or_null()               {
>>> 730:     return method_holder() != nullptr ? method_holder()->jmethod_id_or_null(this) : nullptr;
>> 
>> If `method_holder()` is null at this point what does that mean for the lifecycle of the Method?
>
> Please, ignore this part of code for the time being. I had a crash in CI which was pointing vaguely to this code - unfortunately, the hs_err.log files are not preserved in the test archives and I am not able to reproduce the failure locally. I need to debug the crash and make sure I understand the root cause.

_Update:_ I was able to get to the bottom of the methods not having method holder associated with them.

The `ClassFileParser` does not finalize initialization of the `InstanceKlass` it has created if `_klass != nullptr` (https://github.com/openjdk/jdk/blob/9727f4bdddc071e6f59806087339f345405ab004/src/hotspot/share/classfile/classFileParser.cpp#L5161). This also means, that the `Method` instances are not wired to their method holders via 'constant method'->'constant pool'->'pool holder' chain. However, they need to be deallocated and as such I really need a distinguishing argument for `InstanceKlass::deallocate_methods` call such that I don't attempt to resolve `jmethodid` values in that case.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16662#discussion_r1396296501
PR Review Comment: https://git.openjdk.org/jdk/pull/16662#discussion_r1396175846