RFR: 8305903: Deflate monitors of dead objects before they become unreachable

[Kim Barrett]

On Fri, 28 Apr 2023 14:51:54 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

> With compact object headers ([JDK-8305895](https://bugs.openjdk.org/browse/JDK-8305895)), I've seen occasional failures in G1, where the refinement thread tries to parse a heap region that has dead objects, and would sometimes see an object with a monitor that has already been deflated. And because deflation does not bother to restore the header of dead objects, when heap iteration tries to load the Klass* of the dead object, it would reach to unknown memory and crash.
> 
> In OM::deflate_monitor() we check object_peek(), and if that returns null, then the object header is not updated (and can't be, because the object cannot be reached anymore). Concurrent GCs that process weak handles concurrently ensure that the object doesn't leak out by returning null there (via a barrier). However, for runtime code, at this point, there is no safe way to grab the object and update the header, because the GC might already have reclaimed it. The last safe point in time where we can do that is in WeakProcessor::Task::work() and OopStorage::weak_oops_do() itself, as soon as we detect that the object is properly unreachable.
> 
> The fix is to restore the header of dead objects just before they become unreachable. This can be done in the closures used by WeakProcessor::weak_oops_do(), right before the weak root will be cleared.
> 
> Notice that this is only a bug with compact object headers. It doesn't hurt to fix this in general, though.
> 
> Testing:
>  - [x] tier1
>  - [x] tier2

In particular, during concurrent refinement, we're looking at either parsable
or unparsable parts of a region when processig a card. In the unparsable part,
we don't look at the dead objects for size information to find object
boundaries. Instead we use the mark bits to find live objects, ignoring dead
objects completely. In the parsable part, the dead objects have been
overwritten with filler objects that it is safe to examine. The dead objects
are replaced by fillers concurrently, moving the parsable boundry along the
way. (The replacement by filler objects is an optimization to make card
scanning faster, since no bitmap searching is required to step over one.) At
least, that's all how I think it's supposed to work; it's been a while since I
delved deeply into that code and it's changed somewhat since I last did so.

So I want to better understand the failures being reported, because what's
being described doesn't seem like it should happen and may indicate a bug
elsewhere.

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

PR Comment: https://git.openjdk.org/jdk/pull/13721#issuecomment-1532472878