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

[David Holmes]

On 26/02/2021 3:52 am, Daniel D.Daugherty wrote:
> 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.

True. So I'm unclear how you will reconcile this code with the use of 
get_thread_name() now.

Thanks,
David

>>> ... 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
>