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