8216541: CompiledICHolders of VM locked unloaded nmethods are released too late

[Erik Österlund]

Hi,

There is a bit of an anomaly how we deal with freeing up resources used 
by inline caches when an nmethod dies.

An inline cache might have an associated ICStub if it is in a 
transitional state. But it might also have a CompiledICHolder if it is 
referring to a c2i adapter or an itable stub. These resources need to be 
freed up when an nmethod dies. But they get cleared at completely 
different times.

As for IC stubs, they get "cleared" when is_alive() nmethods get 
converted to zombie. As for nmethods being made unloaded, they will 
(presumably) not have IC caches in transitional states.

As for CompiledICHolders, rather than clearing them out when the nmethod 
dies (transitions to zombie or unloaded), as we did with IC stubs, we 
defer clearing of CompiledICHolders until the nmethod gets deleted, or 
"flushed". Because reasons.

However, unless precaution is taken, it is not safe to find and clear 
out CompiledICHolders when the nmethod gets deleted (as opposed to when 
the nmethod died), due to how we detect there is a CompiledICHolder 
associated with the CompiledIC. We look at the destination of the call 
instruction, and ask said code blob if it is an adapter blob. If it is 
an adapter blob, it is assumed that the CompiledIC must have a 
CompiledICHolder associated with it. What can happen then is that 
between the nmethod dying (in particular due to becoming unloaded), and 
the nmethod being freed, the slot in the CodeCache that the stale 
destination pointed at, has had a c2i adapter allocated over it. So 
asking the question whether a CompiledIC has a CompiledICHolder is not 
safe in general, if the destination is stale, and points at dead 
nmethods. It will then have false positives.

In order to deal with that awkwardness, rather than clearing 
CompiledICHolders when the nmethod dies and it is perfectly safe to 
query whether it has a CompiledICHolder or not, we insist on deferring 
clearing of CompiledICHolders to when the nmethod dies. To make that 
safe and avoid the false positives, the sweeper cleans CompiledICs of 
unloaded nmethods... most of the time...
...except when they are locked in VM. Then we don't do that.

So when the sweeper finds unloaded nmethods that are locked in VM, then 
it skips cleaning of CompiledICs one cycle, its (stale) destination 
possibly gets reclaimed and possibly has a c2i adapter allocated in the 
same memory location as that stale destination points at. When we 
subsequently "flush" the nmethod, we incorrectly find that it has a 
CompiledICHolder (due to its destination pointing at a c2i adapter 
blob), even though it never had a CompiledICHolder when it died. Ouch. 
It goes downhill from there.

It happens to be that when using code loading/unloading JVMTI events, 
this bug is more easily provoked. The reason for this is that the events 
lock the nmethods with the nmethodLocker until some event is being 
processed in the service thread, making the sweeper skip over cleaning 
the ICs of the unloaded nmethods in this corner case of the nmethod life 
cycle.

My proposed solution to this bug is to simply nuke all CompiledIC 
metadata, whether that is ICStubs or CompiledICHolders, the instant the 
nmethod dies (transitions to zombie or unloaded), as well as setting 
state of such CompiledICs to clean. It seems much more fragile to have 
to maintain dead CompiledICs only for the sake of releasing 
CompiledICHolders when freeing the nmethod rather than when it dies. It 
also seems unnecessarily complicated to free CompiledICHolders and 
ICStubs of CompiledICs to get cleared at different times. By just 
clearing all of this when the nmethods die, we remove this complexity.

Bug:
https://bugs.openjdk.java.net/browse/JDK-8216541

Webrev:
http://cr.openjdk.java.net/~eosterlund/8216541/webrev.00/

Going forward, I intend to perform a bunch of cleanups in this area, but 
I wanted to keep the change small ish for the bug fix, to make it easier 
to backport. It will probably need to get backported pretty far back. I 
don't think this has ever worked I'm afraid.

The proposed change has survived 200 rounds of kitchensink, hs-tier1-3 
and hs-precheckin-comp.

Thanks,
/Erik