RFR: 8356173: Remove ThreadCritical [v4]

Coleen Phillimore coleenp at openjdk.org
Wed May 14 12:03:57 UTC 2025


On Tue, 13 May 2025 13:03:31 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Changes requested by kbarrett (Reviewer).
>
> @kimbarrett @coleenp I proposed to use simple global initialization because it makes the code independent from initialization order worries in this sort of very low level very early invoked code. That was the main benefit I had in mind. It is not an immediate concern (we have NMT preinit to deal with early VM time) but every avoided dependency is good.
> 
> The problem with global initialization was on POSIX that pthread_mutex_init() crashes when used at lib loading time on MacOs. The fix for this is to use a static PTHREAD_MUTEX_... initializer, which is also simpler.
> 
> On Windows, one initializes CriticalSections with InitializeCriticalSection(), but that can happen at initialization time in a constructor of a global object. I have done this combination (InitializeCriticalSection on windows, pthread_mutex_t with static initializer on Posix) a lot in many projects, it just works and is simple.
> 
> I am fine with whatever Coleen decides. I don't want to bikeshed this to death, just explain my reasoning.
> 
> P.S. Windows critical sections don't offer recursion checks.

@tstuefe To make the global initialization work, at least on posix, I had to fix the os_posix.cpp code because it was using an uninitialized variable to call pthread_mutex_init.  If I fixed that (check an initialized flag and use the default if it hadn't been initialized), I still had to call pthread_init_common() in os_posix.cpp to guard all the places the code uses these values for _mutexAttr and _condAttr. This did work and enabled me to make the GlobalChunkPoolMutex static.  But then I'd have to fix the windows code, and I don't know windows.  So calling an initialization function when I know the os code is initialized is the platform independent way that I implemented this.  It may be that there might be other reasons to fix this OS code to allow static initialization, but that wasn't the scope of this change.  I just wanted ThreadCritical to be removed.  We were looking at this in the context of other locking changes that are forthcoming, and we want os level locking to be more
  uniform under PlatformMutex.

Thanks for reviewing and you comments Kim, David, Johan and Thomas.

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

PR Comment: https://git.openjdk.org/jdk/pull/25072#issuecomment-2879932655


More information about the hotspot-dev mailing list