RFR: 8304824: NMT should not use ThreadCritical [v2]

[Robert Toyonaga]

On Fri, 6 Sep 2024 20:18:36 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> Robert Toyonaga has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Comments. Hide assertion behind DEBUG. Replace MemoryFileTracker locker. make reentrant.
>
> src/hotspot/share/nmt/nmtCommon.hpp line 168:
> 
>> 166:       intx const current = os::current_thread_id();
>> 167:       intx const owner = Atomic::load(&_owner);
>> 168:       assert(current == owner, "NMT lock should be acquired in this section.");
> 
> How about we try to re-use `assert_locked()` in constructor and destructor, something like:
> 
> 
>     NmtGuard() {
>       assert(!assert_locked(), "Lock is not reentrant");
>       _nmt_semaphore.wait();
>       Atomic::store(&_owner, current);
>     }
> 
>     ~NmtGuard() {
>       assert(assert_locked(), "NMT lock should be acquired in this section.");
>       Atomic::store(&_owner, (intx) -1);
>       _nmt_semaphore.signal();
>     }
> 
>     static bool assert_locked(){
>       intx const current = os::current_thread_id();
>       intx const owner = Atomic::load(&_owner);
>       return (current == owner);
>     }

I think it's a good idea to use it in the destructor. But based on the discussion above, it seems like we shouldn't assert the lock can't be reentered.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20852#discussion_r1748754688