RFR: 8351444: Shenandoah: Class Unloading may encounter recycled oops [v3]

[William Kemper]

On Wed, 12 Mar 2025 13:37:15 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Class unloading is the last thing we do before recycling trash regions. A region will be usable for allocation as soon as it is recycled, so, in a sense, this has the same effect as turning off the weak roots flag immediately after class unloading. 
>> 
>>  Also, the weak roots phase itself cannot have regions recycled because it relies on accurate mark information (recycling clears live data and resets the TAMS). We _could_ work around this by preserving the mark data (perhaps decoupling TAMS reset from region recycling). But changing the `gc_state` currently requires either a safepoint or a handshake (while holding the `Thread_lock`). I haven't thought all the way through this, but something like this (psuedo-code) might be possible:
>> 
>> ```C++
>> vmop_entry_final_mark();
>> 
>> // Complete class unloading, since it actually _needs_ the oops (still need to forbid trash recycling here).
>> entry_class_unloading();
>> 
>> // Recycle trash, but do not reset TAMS (weak roots needs TAMS to decide reachability of referents).
>> entry_cleanup_early();
>> 
>> // Complete weak roots. There are no more trash regions and we don't have to change gc_state
>> entry_weak_refs();
>> entry_weak_roots();
>> 
>> What do you think? This would be a separate PR of course, but do you see any reason something like this wouldn't work? I'd expect some asserts to break if we allocate into a new region with TAMS > bottom.
>
>> A region will be usable for allocation as soon as it is recycled, so, in a sense, this has the same effect as turning off the weak roots flag immediately after class unloading.
> 
> Right. This answers my original question.
> 
>> What do you think? This would be a separate PR of course, but do you see any reason something like this wouldn't work?
> 
> It looks to me as stretching the definition of "trash" even further? I think it would be conceptually cleaner to never turn regular regions into trash until after weak roots are done. So accesses to "dead" weak roots are still possible like a regular access to "regular" region.

The advantage with the scheme I proposed is that it makes immediate trash regions available for allocations earlier in the cycle. I don't think it changes the way "trash" is treated during concurrent class unloading, but it would mean that weak roots/refs wouldn't see "trash" regions any more.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23951#discussion_r1991914955