RFR: 8241403: JavaThread::get_thread_name() should be ThreadSMR-aware [v4]

[Daniel D.Daugherty]

On Wed, 24 Feb 2021 00:42:11 GMT, David Holmes <dholmes at openjdk.org> 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