RFR: 8256811: Delayed/missed jdwp class unloading events [v17]

[Chris Plummer]

On Wed, 13 Jul 2022 13:20:45 GMT, Zhengyu Gu <zgu at openjdk.org> wrote:

>> Ok. Everything is passing after your latest fix.
>> 
>> I think the test still has a synchronization bug, but probably not something important enough to worry about, and I'm guessing a lot of tests have this issue. The test disables events and then frees the raw monitor used in the callback. The problem is that JVMTI might already be in the process of calling the callback, so the callback will still attempt to use the monitor. The test shutdown code and the test callback code need to be synchronized to avoid this issue. However, there is a chicken and egg problem here, because you need a raw monitor for the synchronization. I think the real solution is to just not bother freeing up the raw monitor. I think we may have done this recently in some other test. Perhaps @sspitsyn remembers.
>
>> Ok. Everything is passing after your latest fix.
>> 
>> I think the test still has a synchronization bug, but probably not something important enough to worry about, and I'm guessing a lot of tests have this issue. The test disables events and then frees the raw monitor used in the callback. The problem is that JVMTI might already be in the process of calling the callback, so the callback will still attempt to use the monitor. The test shutdown code and the test callback code need to be synchronized to avoid this issue. However, there is a chicken and egg problem here, because you need a raw monitor for the synchronization. I think the real solution is to just not bother freeing up the raw monitor. I think we may have done this recently in some other test. Perhaps @sspitsyn remembers.
> 
> Me too have a concern on synchronization for enabling/disabling a JVMTI event:
> While setting event bits is synchronized with `JvmtiThreadState_lock` in [JvmtiEventControllerPrivate::set_user_enabled()](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/prims/jvmtiEventController.cpp#L889), but reading [`is_enabled()](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/prims/jvmtiEnvBase.hpp#L308) is unsynchronized. 
> 
> I don't know how it supposes to work, but at least [`_enabled_bits`](https://github.com/openjdk/jdk/blob/master/src/hotspot/share/prims/jvmtiEventController.hpp#L85) should be an `volatile`?

@zhengyu123 I'll be OOO until tuesday, so after this evening I won't be able to respond. @sspitsyn will look at the JVMTI changes again and also help with some of your synchronization questions.

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

PR: https://git.openjdk.org/jdk/pull/9168