On Thu, 23 Nov 2023 14:15:54 GMT, Thomas Stuefe <stu...@openjdk.org> wrote:
>> @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). @tstuefe > 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). Yes. This is intended. The deallocation path via Metaspace is fine. It is just the code that is purging previous versions of a redefined class that has this bug. ------------- PR Comment: https://git.openjdk.org/jdk/pull/16662#issuecomment-1824641628
