On Wed, 24 Feb 2021 00:42:11 GMT, David Holmes <[email protected]> wrote:
>> Holding the Threads_lock is no longer being considered as >> proper protection for the JavaThread::get_thread_name() call >> so I had to change the code to use a JavaThreadIteratorWithHandle >> so that we have the protection of the TLH. That closure eventually >> gets us down to a JavaThread::get_thread_name() call... > > But the code is now broken as it needs the Threads_lock to guard against > non-JavaThreads being created or terminating. > The code also appears misleading because the passing of the JTIWH makes it > look (to me) like it only deals with JavaThreads. > I think this code needs to be: > JavaThreadIteratorWithHandle jtiwh; // ensure JavaThreads are safe to > access > MutexLocker ml(Threads_lock); // ensure non_JavaThreads are safe > to access > Threads::threads_do(&jtiwh, &vmtcc); // process all threads I'm backing out the changes to src/hotspot/share/services/management.cpp. By removing the Threads_lock usage, I'm allowing those closures to run in parallel with other closures that are using the original Threads::java_threads_do() and Threads::threads_do() calls that do use the Threads_lock. Those other uses might be assuming exclusivity for their algorithms and my M&M changes break that. >> ... that will tell us that we have a spot in the code where a TLH is >> missing once we've analyzed the crash. So are you okay with this >> assert() having a bug since the original assert has the same bug? > > Yes It has occurred to me that I should have moved all of the protection checking logic and assertion checks into the new Thread::is_JavaThread_protected() function. That will allow JavaThread::get_thread_name() to become very simple. ------------- PR: https://git.openjdk.java.net/jdk/pull/2535
