RFR: 8346123: [REDO] NMT should not use ThreadCritical [v12]

[Robert Toyonaga]

On Tue, 7 Jan 2025 04:40:36 GMT, David Holmes <dholmes at openjdk.org> wrote:

>>> In case my comment within the existing conversations gets missed I will re-state that I don't think you need any new "is bootstrapping" logic because you can just use `Threads::number_of_threads() > 0` to tell you if you need to acquire the NMT lock. Though that assumes that the `WatcherThread` does not use NMT ... but in that case you can use `WatcherThread::watcher_thread() != nullptr` as the bootstrap condition instead.
>> 
>> I think that the `WatcherThread` does use NMT (at least upon starting), since `Thread::call_run() ` calls `register_thread_stack_with_NMT()`. However, it looks like `WatcherThread::stop()` is called early in `before_exit` -- after which point NMT can still  be used. So checking `WatcherThread::watcher_thread() != nullptr` may not work in this case as it could be set back to nullptr too early.
>> 
>> Another solution might be to introduce something like `Threads::is_single_threaded()` which checks for  `WatcherThread`, `Threads::number_of_threads()`, and any other threads that fall outside this set. I'm not sure whether that would be better than the current approach.
>
>> However, it looks like `WatcherThread::stop()` is called early in `before_exit` -- after which point NMT can still be used. 
> 
> Yeah good catch. I just find it frustrating that we already have so many initialization-progress markers but we are looking at adding yet-another-one. Also I'm not sure this is really relevant to mutex itself per-se it is rather about whether mutexes are needed - to that end I don't find `Threads::is_single_threaded() ` a terrible suggestion.

@dholmes-ora I don't think it is enough to check whether we are single threaded.  Some GHA tests are failing with 


Internal Error (d:\a\jdk\jdk\src\hotspot\share\nmt/memTracker.hpp:68), pid=5708, tid=1620
#  assert(Threads::is_single_threaded() || NmtVirtualMemory_lock->owned_by_self()) failed: should have acquired NmtVirtualMemory_lock


Say **thread_A** is the only thread started. It does not acquire the lock (since single threaded) and enters the critical section. While **thread_A** is in the critical section, **thread_B** is started, acquires the lock (no longer single threaded), and joins **thread_A** in the critical section. The spontaneous creation of **thread_B** creates a race and also causes **thread_A** to trigger assertions checking that it is owner of the lock. 

Maybe the old approach is safer since we always acquire the lock as soon as bootstrapping makes it possible to do so.

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

PR Comment: https://git.openjdk.org/jdk/pull/22745#issuecomment-2580756863