RFR: 8295724: VirtualMachineError: Out of space in CodeCache for method handle intrinsic [v6]

[Martin Doerr]

On Tue, 8 Nov 2022 07:08:47 GMT, Erik Österlund <eosterlund at openjdk.org> wrote:

>> src/hotspot/share/code/nmethod.cpp line 512:
>> 
>>> 510:       nm->oops_do(&cfo);
>>> 511:       assert(!cfo.found_oop(), "no oops allowed");
>>> 512:       CheckForMetadataClosure cfm(/* ignore reference to own Method */ nm->method());
>> 
>> If we are going to ignore method(), then how about checking that it's classloader is parmanent (ClassLoaderData::is_permanent_class_loader_data)?  We also might need a review by a GC expert.  @fisk, do you agree it is safe to put nmethod in NonNMethod space given the above checks?
>
> Hmm. The GC used to walk all nmethods an ensure the is_unloading_state is computed for all of them. When using a STW collector, we will crash if trying to compute this outside of the UnloadingScope, as we then don't have rnough information to be able to compute it. So I suppose this is a bit fragile; if an nmethod is acquired from outside of an nmethod iterator, and the is_unloading() question is asked, we risk crashing.
> I suppose we could strengthen this by letting is_unloading() know rhat the answer is false for method handle intrinsics.
> It feels more and more like method handle intrinsics shouldn't be nmethods as more and more code that deals with nmethods needs to know that this isn't really an nmethod. Although that's not a new observation for this patch.

So, calling `IsUnloadingBehaviour::is_unloading` is unsafe. Should we only call it when `is_permanent_class_loader_data()` for the method returns false?

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

PR: https://git.openjdk.org/jdk/pull/10933