RFR: 8258380: [JVMCI] don't clear InstalledCode reference when unloading JVMCI nmethods

[Erik Österlund]

On Tue, 15 Dec 2020 07:00:40 GMT, Tom Rodriguez <never at openjdk.org> wrote:

> We have been getting crashes running our unit tests in our gates with JDK15. The crashes are mostly in nmethod::is_unloaded because IsUnloadingBehaviour::current() is null. It seemed like there was some inconsistency in the notion of is_unloading for JVMCI nmethods that was leading to the crashes. I noticed that the is_unloading path rescans the nmethod using a closure to determine if it's unloading and since make_unloaded will clear the InstalledCode reference at the end if you ran the closure again it might return false since the oop is now null instead of being an unmarked oop. There's no point in the clearly the oop in this path anyway since the nmethod is dead and leaving the oop around seem to solve this crash. We're only see this crash in 15 but I don't see why it couldn't occur in 11 or later.

Right. It is definitely assumed that the transition to is_unloading() is completely monotonic. And it seems like the clearing could violate that indeed.
Worth noticing is that with concurrent class unloading, the make_unloaded operation is split into two: 1. unlinking, 2. deleting stuff. This operation would go under unlinking. Hence, it would need to check for is_unloading instead of is_unloaded, as the unlinking happens before the nmethod becomes unloaded. But we can cross that bridge when we get there I suppose. Looks good.

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

Marked as reviewed by eosterlund (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1777