RFR: 8297286: runtime/vthread tests crashing after JDK-8296324 [v9]

[Patricio Chilano Mateo]

On Thu, 30 Mar 2023 16:33:01 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> I've just pushed my update with refactoring of these corrections in `VM_SetNotifyJvmtiEventsMode` after discussion of this code with Leonid. I hope it resolved at least part of your concerns.
>> This kind of problem exists only when we disable+enable notifyJvmti events multiple times for testing purposes.
>> This code with corrections of jt->jvmti_thread_state() is not needed when we enable notifyJvmti events just once.
>> Your are correct that the ability to disable notifyJvmti events is implemented with using WhiteBox API.
>> My last update has a comment explaining this.
>> 
>>> In that case can we only execute this restore code when using the WhiteBox API?
>> 
>> This is a good suggestion which we already discussed privately with Leonid.
>> I've got an idea how to implement this check.
>
>> Also not sure if applying this restore in all cases will be correct, since we might be somewhere at a transition. For example, a thread could have blocked right in the return from notifyJvmtiUnmount() in yieldContinuation(). It will looked like virtual because unmount() was not executed yet, and the jvmti_thread_state should be that of the platform thread because we never changed it when mounting. We should leave the state as is but in here we would change it to the virtual thread's jvmti state. The only case I think it makes sense to do this restore steps when enabling the flag is for those threads that are outside a transition with a mounted virtual thread, since we want to adjust the jvmti_thread_state so that it looks right on the next unmount.
> 
> Agreed, thanks. In fact, I've already experimented with it.
> As you noted, this correction is wrong for unmount transition. Also, we don't need to correct it for mount transition because it will be corrected by the `JvmtiVTMSTransitionDisabler::VTMS_mount_end()` as it makes a call `thread->rebind_to_jvmti_thread_state_of(vt)`.
> The only thing that bothers me here is that we fight non-real problems as this code is needed only to support artificial disabling for testing purposes to be able to repeat enable again. :-)

The fix to only restore the state outside a transition looks good.
Yes, the added testing methods are making this harder :)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13133#discussion_r1153670911