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

I agree with Stefan that it seems wrong to be putting object monitor cleaning code into the generic OopStorage and
WeakProcessor code.

> Of course, because the object is now gone, the `deflate_monitor()` code can't fix the header and that didn't used to be a problem. Well actually it's not really a problem in the current mainline, but will be with Lilliput.

Why do we need to fix the header of a dead object?  It's dead.  Who cares what's in the header?  Nobody should
be touching dead objects.  Yes, I know there is heap walking stuff that does that, which is arguably a bug.  If its a
choice between fixing that or doing something like this, well, I'd really like to not do this.

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

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