RFR: 8271348: Add stronger sanity check of thread state when polling for safepoint/handshakes [v2]

[David Holmes]

On Thu, 29 Jul 2021 23:52:50 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> src/hotspot/share/runtime/safepointMechanism.cpp line 122:
>> 
>>> 120:   do {
>>> 121:     JavaThreadState state = thread->thread_state();
>>> 122:     guarantee(SafepointSynchronize::is_a_block_safe_state(state), "Illegal threadstate encountered: %d", state);
>> 
>> Shouldn't this be outside the loop? Though we are doubling up given the assert in block().
>> Do we really want guarantee rather than assert? Doesn't a failure here indicate an internal programming error?
>
> I think the thread's state can change during this loop so having the check in the
> loop will catch if it changes to something bad.
> 
> The assert() in `block()` is to catch any future new callers to `block()` that aren't
> calling from the right thread_state.
> 
> Yes, I specifically wanted a `guarantee()` here to catch this condition in 'release' bits.
> The original internal programming bug was racy and I want to make sure we have the
> best chance to catch any future racy uses.

I would expect any state change to restore the original state before we get back into this loop. So this seems a little paranoid, but okay I guess.

-------------

PR: https://git.openjdk.java.net/jdk/pull/4936