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