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

[Stefan Karlsson]

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

Is there another way to solve this issue? It looks really wrong to put object monitor cleaning code into the generic OopStorage / WeakProcessor code. Could this be pushed out to the calling GC code instead?

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

Changes requested by stefank (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/13721#pullrequestreview-1410174754