RFR: 8377048: Shenandoah: shenandoahLock related improvments

[Xiaolong Peng]

On Tue, 3 Feb 2026 17:39:52 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:

>> While I was working on https://bugs.openjdk.org/browse/JDK-8373371, I made these changes to the shenandoahLock, but I feel it make sense peel off these changes into a separate PR as they are not really related to JDK-8373371. 
>> 
>> Major thing in this PR is to use template for ShenandoahReentrantLock and ShenandoahLocker so we could get rid of duplicate code, along with some other changes where the locks are used e.g. use type alias for ShenandoahNMethodLock and ShenandoahNMethodLocker, use ShenandoahReentrantLock for ShenandoahRebuildLock.
>> 
>> Another improvement is to strengthen assert in SheandnoahHeapLocker to avoid potential dead lock(this was also discussed in previous PR [here](https://github.com/openjdk/jdk/pull/27612#discussion_r2479898169) and our internal channel
>> 
>> ### Test
>> - [x] MacOS aach64, server fastdebug: hotspot_gc_shenandoah
>> - [x] GHA
>
> src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp line 2846:
> 
>> 2844: }
>> 2845: 
>> 2846: ShenandoahHeapLocker::ShenandoahHeapLocker(ShenandoahHeapLock* lock, bool allow_block_for_safepoint) : _lock(lock) {
> 
> Can we have a comment to explain when lock will equal nullptr?

Good catch. 
I think it is impossible to be nullprt. I copied the Locker impl from shenandoahLock.hpp which implies the lock could be nullptr, but I think we should assert non-null for all the Locker impls.

> src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp line 2848:
> 
>> 2846: ShenandoahHeapLocker::ShenandoahHeapLocker(ShenandoahHeapLock* lock, bool allow_block_for_safepoint) : _lock(lock) {
>> 2847:   ShenandoahFreeSet* free_set = ShenandoahHeap::heap()->free_set();
>> 2848:   assert(free_set == nullptr || !free_set->rebuild_lock()->owned_by_self(), "Dead lock, can't acquire heap lock while holding free-set rebuild lock");
> 
> Can we add a comment to explain when free_set will equal nullptr?  Does this correspond to "pre-initialized state" or something else?
> 
> Would probably be more clear that we don't need to consult free_set unless asserts are enabled, so enclose this code in #ifdef ASSERT

Yes, it is related to initialization, and only happens in the  "pre-initialized state", I'll add some comments for this.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29543#discussion_r2760511873
PR Review Comment: https://git.openjdk.org/jdk/pull/29543#discussion_r2760517897