RFR: 8214302: Allow safely calling is_unloading() on zombie nmethods
per.liden at oracle.com
Tue Dec 4 08:06:19 UTC 2018
On 11/30/18 12:19 PM, Erik Österlund wrote:
> Hi Vladimir,
> On 2018-11-27 01:40, Vladimir Kozlov wrote:
>> Hi Erik,
>> Can you tell if there is any concurrency window where you check
>> is_zombie and store new unloading state - can other thread change
>> nmethod to zombie?
> Good question. Short version is: no.
> Here is the longer version to explain why.
> There are 2 modes of importance. Let's start with the first one - STW
> unloading. With STW unloading, all is_alive && is_unloading() nmethods
> are unlinked and unloaded in the safepoint. That makes races where an
> nmethod is first observed as is_alive() && is_unloading() and
> subsequently observed as is_zombie() impossible.
> The more tricky case is with concurrent unloading. The way I have
> structured this code is to unlink all references to is_unloading()
> nmethods (IC caches and dependency contexts) when the mark end safepoint
> is released, and then perform a global handshake operation with all
> JavaThreads before unloading them. The sweeper never converts is_alive
> && is_unloading() nmethods to zombies; it waits for them to become
> is_unloaded(). So before the global handshake, it is impossible for
> is_unloading() nmethods to racingly become is_zombie(). And
> is_unloading() is calculated for all is_alive() nmethods before taking
> that global handshake, meaning that it will never be recalculated after
> the handshake.
> After that global handshake, is_unloading() nmethods are only observable
> to the iterators, and they will never trigger recomputation of the
> cached is_unloading_state, and hence may not suffer from such races.
>> I also noticed that CodeCache::unloading_cycle() is called twice in
>> this code. Can we cache it in local?
> Yes, sure.
Looks good to me.
>> On 11/26/18 7:30 AM, Erik Österlund wrote:
>>> It is currently not safe to call is_unloading on zombie nmethods,
>>> unless it has been observed to be alive. It should be supported to
>>> make the code less fragile. When encountering a !is_alive() nmethod
>>> that has not had its unloading epoch updated, and ask if it
>>> is_unloading(), the answer is always false. So by adding that,
>>> is_unloading() can always be safely called.
More information about the hotspot-compiler-dev