RFR: 8293252: Shenandoah: ThreadMXBean synchronizer tests crash with aggressive heuristics [v2]

[Ashutosh Mehra]

On Thu, 22 Sep 2022 16:39:15 GMT, William Kemper <wkemper at openjdk.org> wrote:

>> Ashutosh Mehra has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Use HeapAccess instead of RawAccess when iterating the heap objects
>>    
>>    Signed-off-by: Ashutosh Mehra <asmehra at redhat.com>
>>  - Make oop_store_common() as private method and remove extraneous condition
>>    
>>    Signed-off-by: Ashutosh Mehra <asmehra at redhat.com>
>
> src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp line 1340:
> 
>> 1338:   template <class T>
>> 1339:   void do_oop_work(T* p) {
>> 1340:     oop obj = HeapAccess<AS_NO_KEEPALIVE>::oop_load(p);
> 
> After taking a closer look at the LRB code, I think we should `or` in `ON_WEAK_OOP_REF` to the access decorators. With only the `AS_NO_KEEPALIVE` decorator, the LRB will fall through all the cases here and end up evacuating a doomed weak referent (i.e., keeping the object alive). 
> https://github.com/openjdk/jdk/blob/master/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.inline.hpp#L119
> 
> I'm worried that this access here could defeat the logic which is meant to keep weak references from being resurrected. It also looks like if we add in the `ON_WEAK_OOP_REF` decorator, the LRB will handle this case for the object iterator too:
> 
> if (_heap->is_concurrent_weak_root_in_progress() && !_marking_context->is_marked(obj)) {
>         // There may be dead oops in weak roots in concurrent root phase, do not touch them.
>         return;
> }

I missed the fact that the object referred by week root would have already been evacuated by the time we hit is_concurrent_weak_root_in_progress condition.

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

PR: https://git.openjdk.org/jdk/pull/10268