[jdk17] RFR: 8268524: nmethod::post_compiled_method_load_event racingly called on zombie

[Erik Österlund]

On Tue, 22 Jun 2021 12:56:10 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> In the code exercised by JvmtiCodeBlobEvents::generate_compiled_method_load_events, we grab a code cache iterator with the NMethodIterator::only_alive_and_not_unloading mode, under the CodeCache_lock. The idea is to then call post_compiled_method_load_event() on each of these is_alive() nmethods. Surely none of them will be a zombie. Inside of post_compiled_method_load_event() we filter out nmethods that racingly can die, like this:
>> 
>> if (is_not_entrant() && can_convert_to_zombie()) {
>>   return;
>> }
>> 
>> So if the nmethod was dead or is_unloading(), we wouldn't get it into the iterator, and here we explicitly filter out nmethods that can become zombies. Now we should have all bases covered, no way we can end up calling the subsequent code on a zombie!
>> 
>> Except... the code called by the sweeper that flips an nmethod to zombie, doesn't hold the CodeCache_lock. Instead it holds the CompiledMethod_lock, which this JVMTI code does not hold. So between it being alive in the iterator, and calling is_not_entrant(), the nmethod could have racingly already become zombie. So when we check is_not_entrant(), it will return false. Because it's a zombie. Therefore we are tricked into believing the nmethod is safe to post around these events, while in fact it is already dead.
>> 
>> After we have mistakenly grabbed a zombie nmethod, when we use ZGC, we call the nmethod entry barriers on it. It gets indigestion due to being called on a zombie. I'm sure there are more sources of indigestion as well.
>
> src/hotspot/share/code/nmethod.cpp line 1611:
> 
>> 1609:       return;
>> 1610:     }
>> 1611:   }
> 
> Can't we change state from alive -> not_entrant here?  Should this have a mark_as_seen_on_stack somewhere?

not_entrant() is also implicitly is_alive(). That's why we need to check if they are not_entrant() in addition to is_alive(), to see if we are in a scenario where after we release the locks, it may racingly flip to zombie. That only happens for not_entrant nmethods, and we filter out the ones that can happen to by checking can_convert_to_zombie() on those.

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

PR: https://git.openjdk.java.net/jdk17/pull/107