NotifyFramePop request is not cleared if JVMTI_EVENT_FRAME_POP is disabled

A NotifyFramePop request is only cleared if the JVMTI_EVENT_FRAME_POP is enabled but it must be always cleared when the corresponding frame is popped regardless of the JVMTI_EVENT_FRAME_POP is enabled or not.

Please, see the email exchange below:

That is only cleared if the state is enabled for JVMTI_EVENT_FRAME_POP. If you disable it before, you'd skip that.

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?

On Tue, Sep 5, 2017 at 11:05 PM, serguei.spitsyn@oracle.com <serguei.spitsyn@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 <jcbeyler@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