On Fri, 8 Nov 2024 13:43:14 GMT, Patricio Chilano Mateo
<[email protected]> wrote:
>> src/hotspot/share/oops/stackChunkOop.inline.hpp line 189:
>>
>>> 187: inline ObjectMonitor* stackChunkOopDesc::current_pending_monitor()
>>> const {
>>> 188: ObjectWaiter* waiter = object_waiter();
>>> 189: if (waiter != nullptr && (waiter->is_monitorenter() ||
>>> (waiter->is_wait() && (waiter->at_reenter() || waiter->notified())))) {
>>
>> Can we hide this conditional under ObjectWaiter::pending_monitor() { all
>> this stuff with a comment; }
>>
>> Not sure what this is excluding.
>
> I added method `at_monitorenter()` to ObjectWaiter. We are checking if the
> vthread is blocked trying to acquire (or re-acquire for the wait case) the
> monitor. While looking at these I also noticed we were missing a call to
> `current_waiting_monitor` in `JvmtiEnvBase::get_locked_objects_in_frame()` so
> I added it. We didn’t had a case for this so it went unnoticed. I extended
> JVMTI test `VThreadMonitorTest.java` to cover this case.
Thanks for pointing at this code because I also realized there is a nice
cleanup that can be done here. First these methods should be moved to
`java_lang_VirtualThread` class where they naturally belong (these are the
equivalent of the JavaThread methods but for an unmounted vthread). Also the
`objectWaiter` field can be added to the VirtualThread class rather than the
stackChunk, which not only is more appropriate too and gives us the get/set
symmetry for these methods, but we can also save memory since one virtual
thread can have many stackChunks around. I have a patch for that but I’ll do it
after this PR to avoid new changes.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1834429835
