On Tue, 7 Jan 2025 04:40:36 GMT, David Holmes <dhol...@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

Reply via email to