Re: NotifyFramePop and FramePop events

[serguei.spit...@oracle.com]

Fixing it in the spec is wrong. The implementation needs to be fixed. A straight-forward fix will most likely cause some performance degradation in interpreter mode. So, the fix is going to be more tricky. If you do not object, I'll assign this bug to my self. Will see if there is a time to fix it in 10. Thanks, Serguei On 9/6/17 16:56, JC Beyler wrote: Thanks Serguei! Just so I know, is your current thought that we should try to fix the implementation or change the documentation something like I did? Perhaps you don't know yet :) Jc On Wed, Sep 6, 2017 at 3:07 PM, serguei.spit...@oracle.com <serguei.spit...@oracle.com> wrote: On 9/6/17 10:29, JC Beyler wrote: That is only cleared if the state is enabled for JVMTI_EVENT_FRAME_POP. If you disable it before, you'd skip that. Agreed. It is a bug in implementation. When I go look at the jvmtiEventController and peruse, it seems that if you can have the case I am talking about if all the threads turn off the JVMTI_EVENT_FRAME_POP, then the state would be off as well and then we would not clear (or even check for that matter). Am I wrong? You are right. I have filed the bug:   https://bugs.openjdk.java.net/browse/JDK-8187289     NotifyFramePop request is not cleared if JVMTI_EVENT_FRAME_POP is disabled Thanks, Serguei On Tue, Sep 5, 2017 at 11:05 PM, serguei.spit...@oracle.com <serguei.spit...@oracle.com> wrote: Hi JC, Thank you for verifying this! On 9/5/17 11:01, JC Beyler wrote: Hi all, After looking at the code a bit more, I don't see a viable way of really either:    - At notification disabling, remove all pop event requests    - Enforcing that the current frame at depth is singled out, if you disable and then pop that frame, the event is destroyed with the frame so that a subsequent pop at that depth no longer fires Because of that, I'd recommend just changing the documentation to highlight this, it might look a bit like this: --- old/src/share/vm/prims/jvmti.xml 2017-09-05 10:51:05.850810934 -0700 +++ new/src/share/vm/prims/jvmti.xml 2017-09-05 10:51:05.710811367 -0700 @@ -2907,7 +2907,7 @@ <function id="NotifyFramePop" num="20"> <synopsis>Notify Frame Pop</synopsis> <description> - When the frame that is currently at <paramlink id="depth"></paramlink> + When a frame currently at <paramlink id="depth"></paramlink> is popped from the stack, generate a <eventlink id="FramePop"></eventlink> event. See the <eventlink id="FramePop"></eventlink> event for details. @@ -2916,6 +2916,12 @@ <p/> The specified thread must either be the current thread or the thread must be suspended. + <p/> + Note: if the notification event is disabled and a frame at + <paramlink id="depth"></paramlink> is popped, no event is generated. + After re-enabling the notification event, the registered + <paramlink id="depth"></paramlink> is still active and will provoke an + event when a frame at <paramlink id="depth"></paramlink> is popped. </description> <origin>jvmdi</origin> <capabilities> Would someone want to review this and tell me if I should create a webrev for it? Or tell me hell no ;-) I'd say, "hell no" as it looks wrong to me. ;-) Please, see a comment below. Thanks! Jc On Thu, Aug 31, 2017 at 3:15 PM, JC Beyler <jcbey...@google.com> wrote: Hi all, I was asked to raise a question about NotifyFramePop and FramePop events and I thought I would just ask it here: If we imagine we have a stack frame such as: call_depth0   call_depth1     call_depth2        call_depth3 And at this third depth, we request a frame pop when leaving depth1 via the NotifyFramePop call. We would of course assume that when leaving call_depth1 we get a FramePop event. Now imagine that we disable the frame pop event notification in call_depth2: call_depth0   call_depth1     call_depth2        SetEventNotificationMode(JVMTI_DISABLE, JVMTI_EVENT_FRAME_POP, NULL) If the stack now pops back to call_depth0, the frame pop system is not checked, the FramePop for call_depth1 is not issued either. However, imagine now that later down the road, the stack trace has built itself back up and we enabled the event: call_depth0   second_call_depth1     second_call_depth2        SetEventNotificationMode(JVMTI_ENABLE, JVMTI_EVENT_FRAME_POP, NULL) Then when leaving second_call_depth1, we seemingly will issue a frame pop event. No. The NotifyFramePop request has been already cleaned at the first call_depth0 pop. Please, see the following line in the jvmtiExport.cpp: JvmtiExport::post_method_exit():           ets->clear_frame_pop(cur_frame_number); We have to make another NotifyFramePop request to get this one. Thanks, Serguei Here is the qualm:   - It seems counter intuitive and the documentation does not specify/warn about this; it seems that if you disable the events you should perhaps clear up the pop requests.   - At least the documentation for NotifyFramePop (https://docs.oracle.com/javase/7/docs/platform/jvmti/jvmti.html#NotifyFramePop) should be changed since it says: "When the frame that is currently at depth is popped from the stack" to something like "When the first frame at the depth is popped from the stack and the event notification is enabled" Therefore the questions are: 1) Is this such a corner case, that we do not wish to try to fix the documentation or the code? 2) Is this a corner case we do not wish to handle, therefore put a fix in the documentation to at least warn users of this 3) Is this a bug that we'd like to fix? Thanks for your insight, Jc