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

[Aleksey Shipilev]

On Tue, 11 Mar 2025 23:35:24 GMT, William Kemper <wkemper 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.

Some nits:

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)

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?

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

Marked as reviewed by shade (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24001#pullrequestreview-2677892517
PR Review Comment: https://git.openjdk.org/jdk/pull/24001#discussion_r1991165939
PR Review Comment: https://git.openjdk.org/jdk/pull/24001#discussion_r1991163625