RFR: 8350905: Shenandoah: Releasing a WeakHandle's referent may extend its lifetime

[William Kemper]

On Wed, 12 Mar 2025 10:30:58 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> When weak handles are cleared, the `nullptr` is stored with the `ON_PHANTOM_OOP_REF` decorator. For concurrent collectors using a SATB barrier like Shenandoah, this may cause the referent to be enqueued and marked when it would be otherwise unreachable. The problem is especially acute for Shenandoah's generational mode, in which a young region holding the otherwise unreachable referent, may become trash after the referent is enqueued for old marking. Shenandoah's store barrier should be modified to not enqueue WEAK or PHANTOM stores in the SATB buffer.
>
> src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.inline.hpp line 159:
> 
>> 157:       HasDecorator<decorators, AS_NO_KEEPALIVE>::value ||
>> 158:       HasDecorator<decorators, ON_WEAK_OOP_REF>::value ||
>> 159:       HasDecorator<decorators, ON_PHANTOM_OOP_REF>::value) {
> 
> Suggest to split it into two things, with comments:
> 
> 
>   // Uninitialized and no-keepalive stores do not need barrier.
>   if (HasDecorator<decorators, IS_DEST_UNINITIALIZED>::value ||
>       HasDecorator<decorators, AS_NO_KEEPALIVE>::value) {
>     return;
>   }
> 
>   // Stores to weak/phantom require no barrier. The old references would
>   // have been resurrected by load barrier if they were needed.
>   if (HasDecorator<decorators, ON_WEAK_OOP_REF>::value ||
>       HasDecorator<decorators, ON_PHANTOM_OOP_REF>::value) {
>     return;
>   }
> 
> 
> (I think I caught the reason why we are safe to skip SATB here, maybe comment can be expanded)

Ha. I had it that way originally - I'll put it back.

> src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.inline.hpp line 279:
> 
>> 277:   oop_store_common(addr, value);
>> 278:   if (ShenandoahCardBarrier) {
>> 279:     barrier_set()->write_ref_field_post<decorators>(addr);
> 
> Unnecessary change?

Yes, just idly fixing warnings in my editor. I'll revert it.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24001#discussion_r1991898007
PR Review Comment: https://git.openjdk.org/jdk/pull/24001#discussion_r1991899265