RFR: 8313816: Accessing jmethodID might lead to spurious crashes [v2]

[Thomas Stuefe]

On Thu, 23 Nov 2023 08:44:08 GMT, Jaroslav Bachorik <jbachorik at openjdk.org> wrote:

>> Jaroslav Bachorik has updated the pull request incrementally with three additional commits since the last revision:
>> 
>>  - Clean up imports
>>  - Simplify Method::clear_jmethod_id()
>>  - Use correct copyrights
>
> @dholmes-ora 
>> Can't we just check method->method_holder() for null rather than passing in a parameter like this?
> 
> I have removed the argument and I am now performing the check for `method_holder() != nullptr` as recommended. The code is a bit simpler and the cost of resolving the method holder for each method is probably quite low so we should be ok here.

@jbachorik You are aware that this fix only works for some uncommon corner cases, right?

It only works if the Method is explicitly deallocated. The vast bulk of Method aren't. Method, as a Metaspace object, is released in bulk when the class is unloaded. The `::deallocate` path you fixed up - that eventually ends up in `MetaspaceArena::deallocate()` - is a rare case that only gets invoked if
- a class cannot be loaded but parts of it have already been loaded into Metaspace. 
- a class gets transformed

In case the class gets unloaded via conventional means, your fix won't get invoked (nor should it; releasing in bulk without having to care for individual allocations is the point of Metaspace).

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

PR Comment: https://git.openjdk.org/jdk/pull/16662#issuecomment-1824509686