RFR: 8356173: Remove ThreadCritical [v2]

[Coleen Phillimore]

On Fri, 9 May 2025 20:41:03 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add recursion detection.
>
> src/hotspot/share/memory/arena.cpp line 44:
> 
>> 42: // been created so we cannot use Mutex.
>> 43: static PlatformMutex* _global_chunk_pool_mutex = nullptr;
>> 44: static volatile int _recursion_count = 0;
> 
> Why is this volatile?  It's only accessed in the context of the lock.  Also, I think it probably doesn't
> help for POSIX, since we're using `PTHREAD_MUTEX_NORMAL`, for which attempting recursion
> "shall deadlock", with no error check.  (If you want error checking, you need to use
> `PTHREAD_MUTEX_ERRORCHECK` and check the result of the lock attempt.)
> 
> I think the Windows critical sections that are used to implement PlatformMutex do allow recursion,
> and maybe we should have this kind of guard to prevent recursive use there, to be consistent with
> the restriction imposed by the POSIX implementation.  (Or change the POSIX implementation to
> use `PTHREAD_MUTEX_RECURSIVE`, but I don't think we should do that.)
> 
> Maybe the POSIX PlatformMutex should use `PTHREAD_MUTEX_ERRORCHECK` in debug
> builds.  But that's a separate issue.

You're right, it doesn't need to be volatile. I want the recursion checking to be platform independent for this lock, and not affect the other PlatformMutex/Monitors which are outside the scope of this change and hopefully are what we want already.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25072#discussion_r2082561451