On Fri, 15 Oct 2021 21:34:50 GMT, Daniel D. Daugherty <dcu...@openjdk.org> wrote:
>> 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. Should the ThreadsListHandle protect JvmtiThreadState::first also? If state->next() needs it why doesn't the first entry need this? There's no atomic load on the _head field. ------------- PR: https://git.openjdk.java.net/jdk/pull/4677
