RFR: 8318462: [GenShen] Prevent unsafe access to displaced mark-word

[Roman Kennke]

On Wed, 18 Oct 2023 21:53:09 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> We must be very careful when we access (load or update) the object age concurrently because the age is stored in the object's header and that header can be 'displaced'. This means that the header is overloaded with a pointer (stack or ObjectMonitor*) and the original header is stored at that location. The header can also be in INFLATING state, which means that a stack-lock is currently in the process to be inflated to an ObjectMonitor. Let's consider the cases separately:
>> 
>> Loading object age:
>> - INFLATING: we can not access the header, and thus not the age.
>> - Stack-locked: the thread which locks the object can unlock at any time, concurrently. It is not safe to access the header, and thus the age.
>> - Monitor-locked: monitors don't go away without coordinating with Java threads and possibly GC threads. This coordination is done by handshaking the thread - a Java thread would be brought to its next safepoint, and GC threads, which are typically participating in handshakes by joining the SuspendibleThreadSet, can opt to handshake at a safe point by calling SuspendibleThreadSet::yield(). If 'our' thread participates in handshakes in one way or the other, it is safe to access an ObjectMonitor once we got a valid ObjectMonitor* from an object's header, but only until that thread handshakes. Long story short: it is typically safe to access ObjectMonitor :-)
>> 
>> Updating object age:
>> Updating object age only happens during evacuation, and only on a new copy of an object, before that copy has been published. Access to the object header is therefore fully thread-local and not problematic. What *is* problematic is when we have to deal with displaced headers, because displaced headers are *not* thread local, and must be considered a shared resource. A competing thread may succeed to evacuate the object and publish its copy before 'our' thread can do so. If 'our' thread would update the displaced header, it may over-write whatever the other thread has done.
>> - INFLATING: We cannot access the header at all. However, that should not happen: a thread can only inflate once it has successfully evacuated an object, and 
>> - Stack-locked: the other thread may succeed evacuation and continue to unlock the object, at which point the stack-pointer would be 'dangling' and we may over-write random information on the foreign thread stack.
>> - Monitor-locked: even though monitor deflation is coordinated (see above), updating the displaced header in a monitor might over-...
>
> src/hotspot/share/gc/shenandoah/shenandoahHeap.inline.hpp line 569:
> 
>> 567:   markWord w = obj->mark();
>> 568:   if (w.has_monitor()) {
>> 569:     w = w.monitor()->header();
> 
> Isn't this prone to potential error because the monitor might be recycled/reused between line 567 and line 569?

Monitor deflation is coordinated with Java threads by safepointing (or more precisely handshaking) the Java threads. The same works with GC threads if they participate in safepointing/handshaking. This is usually achieved by joining SuspendibleThreadSet. The effect of is that all such threads can safely access monitors without fearing that they go away. When we got a valid monitor pointer, then that monitor will not disappear before the next safepoint, and safepoints cannot happen at random points in execution. For example, a thread that has joined STS would only be able to safepoint when it calls STS::yield(). We might want to verify that all GC threads which can evacuate objects are properly joining (and yielding) STS.

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

PR Review Comment: https://git.openjdk.org/shenandoah/pull/343#discussion_r1365182526