On Thu, 4 Nov 2021 19:33:33 GMT, Coleen Phillimore <cole...@openjdk.org> wrote:
>> 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. The `ThreadsListHandle` protects `JavaThread` objects not `JvmtiThreadState` objects. `JvmtiThreadState::first()` returns the head of the global list of `JvmtiThreadState` objects for the system. Each `JvmtiThreadState` object contains a `JavaThread*` and we have to protect use of the `JavaThread*` which can happen in the `recompute_thread_enabled(state)` call below. ------------- PR: https://git.openjdk.java.net/jdk/pull/4677
