RFR: 8377048: Shenandoah: shenandoahLock related improvments [v3]

[Xiaolong Peng]

On Tue, 3 Feb 2026 18:46:37 GMT, Xiaolong Peng <xpeng at openjdk.org> wrote:

>> 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.

I have changed code to assert _lock != nullptr for both ShenandoahHeapLocker and the ShenandoahLocker<Lock> in shenandoahLock.hpp.

>> src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp line 121:
>> 
>>> 119: 
>>> 120: typedef ShenandoahLock                       ShenandoahHeapLock;
>>> 121: // ShenandoahHeapLocker checks potential deadlock and asserts
>> 
>> Years from now, the reader of this comment will not "appreciate" the context in which it was written.  The comment only makes sense in the context of this "diff".
>> 
>> Would be better to say: ShenandoahHeapLocker implements a lock to assure mutually exclusive access to the global heap data structures.  Asserts in the implementation detect potential deadlock usage with regards the rebuild lock that is present in ShenandoahFreeSet.  Whenever both locks are acquired, this lock should be acquired before the ShenandoahFreeSet rebuild lock.
>
> Thanks, appreciate it! I'll update the comments here.

Updated the comment as you suggested.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29543#discussion_r2762544296
PR Review Comment: https://git.openjdk.org/jdk/pull/29543#discussion_r2762539612