On 16/10/2021 8:26 am, Daniel D.Daugherty wrote:
On Fri, 15 Oct 2021 06:34:42 GMT, David Holmes <dhol...@openjdk.org> wrote:
Daniel D. Daugherty has updated the pull request incrementally with one
additional commit since the last revision:
8249004.cr1.patch
src/hotspot/share/prims/jvmtiEventController.cpp line 623:
621: // If we have a JvmtiThreadState, then we've reached the point where
622: // threads can exist so create a ThreadsListHandle to protect them.
623: ThreadsListHandle tlh;
Good catch on the missing TLH for this code.
It wasn't quite missing from the baseline code. This version of execute():
`Handshake::execute(HandshakeClosure* hs_cl, JavaThread* target)`
used to always create a ThreadsListHandle. I added a `ThreadsListHandle*`
parameter to that version and created a wrapper with the existing signature
to pass `nullptr` to the execute() version with the `ThreadsListHandle*`
parameter. What that means is that all existing callers of:
`Handshake::execute(HandshakeClosure* hs_cl, JavaThread* target)`
no longer had a ThreadsListHandle created for them. With the new sanity
check in place, I shook the trees to make sure that we had explicit
ThreadsListHandles in place for the locations that needed them.
`JvmtiEventControllerPrivate::recompute_enabled()` happened to be
one of the places where the ThreadsListHandle created by execute()
was hiding the fact that `recompute_enabled()` needed one.
Yup and that is exactly why I said good catch on finding the missing TLH.
Cheers,
David
src/hotspot/share/prims/jvmtiEventController.cpp line 624:
622: // threads can exist so create a ThreadsListHandle to protect them.
623: ThreadsListHandle tlh;
624: for (; state != NULL; state = state->next()) {
s/NULL/nullptr/
Missed that one. Fixed.
src/hotspot/share/runtime/handshake.cpp line 361:
359: } else {
360: if (tlh_p == nullptr) {
361: guarantee(Thread::is_JavaThread_protected_by_my_ThreadsList(target),
This should be an assert once this has had some bake time.
Agreed. All of the
`guarantee(Thread::is_JavaThread_protected_by_my_ThreadsList(),...`
calls should be changed to asserts down the road.
-------------
PR: https://git.openjdk.java.net/jdk/pull/4677
