On Wed, 15 Nov 2023 18:47:19 GMT, Jaroslav Bachorik <jbacho...@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
