On Wed, 23 Nov 2022 10:14:23 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:
>> This problem has two sides. >> One is that the `VirtualThread::run() `cashes the field `notifyJvmtiEvents` >> value. >> It caused the native method `notifyJvmtiUnmountBegin()` not called after the >> field `notifyJvmtiEvents` >> value has been set to `true` when an agent library is loaded into running VM. >> The fix is to get rid of this cashing. >> Another is that enabling `notifyJvmtiEvents` notifications needs a >> synchronization. >> Otherwise, a VTMS transition start can be missed which will cause some >> asserts to fire. >> The fix is to use a JvmtiVTMSTransitionDisabler helper for sync. >> >> Testing: >> The originally failed tests are passed now: >> >> runtime/vthread/RedefineClass.java >> runtime/vthread/TestObjectAllocationSampleEvent.java >> >> In progress: >> Run the tiers 1-6 to make sure there are no regression. > > Serguei Spitsyn has updated the pull request incrementally with one > additional commit since the last revision: > > remove caching if notifyJvmtiEvents in yieldContinuation I've forgotten the `JvmtiVTMSTransitionDisabler` is not going to work before the `notifyJvmtiEvents` is set to `true`. I agree, we may want to allow `start_VTMS_transition/finish_VTMS_transition` not properly paired as you suggest. But then it is not good that we loose the ability to strictly check/assert pairing of VTMS transition notifications for other cases. Need to think a bit more on this. ------------- PR: https://git.openjdk.org/jdk/pull/11304
