On Thu, 7 Nov 2024 00:37:17 GMT, Patricio Chilano Mateo <pchilanom...@openjdk.org> wrote:
>> Thank you for the comment! I'm okay with your modified suggestion in general >> if there are no road blocks. >> >>> but does the specs say the event has to be posted in the context of the >>> vthread? >> >> As Alan said below we do not have an official spec for this but still the >> events need to be posted in vthread context. >> >>> For pop_frame/early_ret checks ... >> >> The pop_frame/early_ret conditions are installed in handshakes with a >> context of `JvmtiVTMSTransitionDisabler`. As you noted the >> `JVMTI_ERROR_OPAQUE_FRAME` might be also returned by the JVMTI `FramePop` >> and `ForceEarlyReturn*` for some specific cases. So, it feels like it should >> not be a problem. I'm thinking if adding an assert at the VTMS transition >> end would help. >> >>> Maybe we could go with this simplified code now and work on it later... >> >> Whatever works better for you. An alternate approach could be to file an >> enhancement to simplify/refactor this. >> It would be nice to fix a couple of nits though: >> - the call to `java_lang_Thread::set_is_in_VTMS_transition()`is not needed >> in `JvmtiUnmountBeginMark` >> - the function `is_vthread_safe_to_preempt()` does not need the `vthread` >> parameter > > Great, I applied the suggested simplification. I had to update test > `VThreadEventTest.java` to check the stack during the mount/unmount events to > only count the real cases. This is because now we are getting a variable > number of spurious mount/unmount events (freeze failed) generated during the > initialization of some class (`VirtualThreadEndEvent`) after the task is > finished. > So, it feels like it should not be a problem. I'm thinking if adding an > assert at the VTMS transition end would help. > The problem here is that for monitorenter the top frame will not be a native method, so the bail out will not happen as it would when unmounting from Java. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1831898439
