RFR: 8356173: Remove ThreadCritical

[Thomas Stuefe]

On Fri, 9 May 2025 14:50:08 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Yes, this is needed early.  We create the ResourceArea arena when creating a JavaThread before the JavaThread is completely created so Thread::current() doesn't exist in this case.  There are also other early initializations.
>> 
>> There's two things that make us unable to have a static PlatformMutex.  1. on macosx, the PlatformMutex is implemented using an indirection the macosx dll load crashes trying to initialize it.
>> 
>> 
>> #  assert(status == 0) failed: error EINVAL(22), freelist lock
>> 
>> V  [libjvm.dylib+0xea68c4]  PlatformMutex::PlatformMutex()+0xfc
>> V  [libjvm.dylib+0x269c34]  _GLOBAL__sub_I_arena.cpp+0x38
>> C  [dyld+0x22a24]  invocation function for block in dyld4::Loader::findAndRunAllInitializers(dyld4::RuntimeState&) const::$_0::operator()() const+0xa8
>> C  [dyld+0x680f4]  invocation function for block in dyld3::MachOAnalyzer::forEachInitializer(Diagnostics&, dyld3::MachOAnalyzer::VMAddrConverter const&, void (unsigned int) block_pointer, void const*) const+0xac
>> C  [dyld+0x5b668]  invocation function for block in dyld3::MachOFile::forEachSection(void (dyld3::MachOFile::SectionInfo const&, bool, bool&) block_pointer) const+0x1f0
>> C  [dyld+0x22fc]  dyld3::MachOFile::forEachLoadCommand(Diagnostics&, void (load_command const*, bool&) block_pointer) const+0x12c
>> C  [dyld+0x5a6a0]  dyld3::MachOFile::forEachSection(void (dyld3::MachOFile::SectionInfo const&, bool, bool&) block_pointer) const+0xc0
>> C  [dyld+0x5d188]  dyld3::MachOFile::forEachInitializerPointerSection(Diagnostics&, void (unsigned int, unsigned int, bool&) block_pointer) const+0xa0
>> C  [dyld+0x67de8]  dyld3::MachOAnalyzer::forEachInitializer(Diagnostics&, dyld3::MachOAnalyzer::VMAddrConverter const&, void (unsigned int) 
>> 
>> 
>> 2. Even if I disable the indirect PlatformMutex workaround for the macosx bug JDK-8218975, the underlying pthread_mutex is not initialized before this code uses it.
>> 
>> 
>> #  assert(status == 0) failed: error EINVAL(22), mutex_init
>> 
>> 
>> So I don't think PlatformMutex can be a static variable, which is why I initialize it early in vm startup.
>
> I added the initialization of the mutex after os::init().  Assuming that pthread_mutex will be initialized at that point.

@coleenp But surely one can statically allocate a pthread_mutex_t and assign PTHREAD_MUTEX_INITIALIZER, no? I mean that's Unix 101. MacOS should be able to do that at least.

Looking at PlatformMutex, maybe it is not the right tool for the job. One needs a simple stupid mutex class like this:


struct SimpleMutex {
    pthread_mutex_t _m;
    SimpleMutex() : _m(PTHREAD_MUTEX_INITIALIZER) {}
    lock() -> do pthread_mutex_lock
    unlock() -> do pthread_mutex_unlock
};
static SimpleMutex mymutex;


etc.

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

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