Skip to content
Permalink
Browse files

8244416: Remove incorrect assert during inline cache cleaning

Reviewed-by: kvn, pliden
  • Loading branch information
fisk committed May 20, 2020
1 parent afe76a9 commit 3f7cd1f20460c6a5ccb869a3f3893b6a3a5a9173
Showing with 27 additions and 5 deletions.
  1. +27 −5 src/hotspot/share/code/compiledMethod.cpp
@@ -492,17 +492,39 @@ static bool clean_if_nmethod_is_unloaded(CompiledICorStaticCall *ic, address add
if (clean_all || !nm->is_in_use() || nm->is_unloading() || (nm->method()->code() != nm)) {
// Inline cache cleaning should only be initiated on CompiledMethods that have been
// observed to be is_alive(). However, with concurrent code cache unloading, it is
// possible that by now, the state has been racingly flipped to unloaded if the nmethod
// being cleaned is_unloading(). This is fine, because if that happens, then the inline
// possible that by now, the state has become !is_alive. This can happen in two ways:
// 1) It can be racingly flipped to unloaded if the nmethod // being cleaned (from the
// sweeper) is_unloading(). This is fine, because if that happens, then the inline
// caches have already been cleaned under the same CompiledICLocker that we now hold during
// inline cache cleaning, and we will simply walk the inline caches again, and likely not
// find much of interest to clean. However, this race prevents us from asserting that the
// nmethod is_alive(). The is_unloading() function is completely monotonic; once set due
// to an oop dying, it remains set forever until freed. Because of that, all unloaded
// nmethods are is_unloading(), but notably, an unloaded nmethod may also subsequently
// become zombie (when the sweeper converts it to zombie). Therefore, the most precise
// sanity check we can check for in this context is to not allow zombies.
assert(!from->is_zombie(), "should not clean inline caches on zombies");
// become zombie (when the sweeper converts it to zombie).
// 2) It can be racingly flipped to zombie if the nmethod being cleaned (by the concurrent
// GC) cleans a zombie nmethod that is concurrently made zombie by the sweeper. In this
// scenario, the sweeper will first transition the nmethod to zombie, and then when
// unregistering from the GC, it will wait until the GC is done. The GC will then clean
// the inline caches *with IC stubs*, even though no IC stubs are needed. This is fine,
// as long as the IC stubs are guaranteed to be released until the next safepoint, where
// IC finalization requires live IC stubs to not be associated with zombie nmethods.
// This is guaranteed, because the sweeper does not have a single safepoint check until
// after it completes the whole transition function; it will wake up after the GC is
// done with concurrent code cache cleaning (which blocks out safepoints using the
// suspendible threads set), and then call clear_ic_callsites, which will release the
// associated IC stubs, before a subsequent safepoint poll can be reached. This
// guarantees that the spuriously created IC stubs are released appropriately before
// IC finalization in a safepoint gets to run. Therefore, this race is fine. This is also
// valid in a scenario where an inline cache of a zombie nmethod gets a spurious IC stub,
// and then when cleaning another inline cache, fails to request an IC stub because we
// exhausted the IC stub buffer. In this scenario, the GC will request a safepoint after
// yielding the suspendible therad set, effectively unblocking safepoints. Before such
// a safepoint can be reached, the sweeper similarly has to wake up, clear the IC stubs,
// and reach the next safepoint poll, after the whole transition function has completed.
// Due to the various races that can cause an nmethod to first be is_alive() and then
// racingly become !is_alive(), it is unfortunately not possible to assert the nmethod
// is_alive(), !is_unloaded() or !is_zombie() here.
if (!ic->set_to_clean(!from->is_unloading())) {
return false;
}

0 comments on commit 3f7cd1f

Please sign in to comment.
You can’t perform that action at this time.