[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: JDK-8187289: NotifyFramePop request is not cleared if JVMTI_EVENT_FRAME_POP is disabled
From:       "serguei.spitsyn () oracle ! com" <serguei ! spitsyn () oracle ! com>
Date:       2018-05-29 20:59:21
Message-ID: d18e0716-d641-7249-ca01-c9ac91942bed () oracle ! com
[Download RAW message or body]

Hi Dan and Chris,

You both already reviewed my version of the fix below.
For your convenience, my webrev was:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2017/hotspot/8187289-jvmti-framepop.2/
I did not push it into 10 because some number of regressions were 
discovered.
Thanks a lot to Alex for isolating and finding how to fix those regressions!

The fix from Alex has a minor change since that time.
I think, the difference is in the jvmtiExport.cpp.

I know you are/were busy or seek these days.
Just one more review is needed.
There is no big hurry, it can wait if necessary.

Thanks,
Serguei


On 5/23/18 15:19, Alex Menkov wrote:
> Hi Serguei,
>
> Thank you for the review.
>
> The issue is described in CSR in details.
> Short summary:
> A NotifyFramePop request is only cleared if the JVMTI_EVENT_FRAME_POP 
> is enabled. The problem is that if a NotifyFramePop request is not 
> cleared when the JVMTI_EVENT_FRAME_POP events notification is 
> disabled, then an unrelated FRAME_POP event is sent after the 
> notification is re-enabled (test in the fix demonstrates the scenario).
>
> Changes in jvmtiExport.cpp updates updates methodExit handler to clear 
> pending FramePop events even if JVMTI_EVENT_FRAME_POP is disabled.
> Changes in jvmtiEventController.cpp turns on interp_only_mode if 
> NotifyFramePop is requested (this is also for the case when 
> JVMTI_EVENT_FRAME_POP is disabled).
>
> --alex
>
> On 05/23/2018 13:58, serguei.spitsyn@oracle.com wrote:
>> Hi Alex,
>>
>> Looks good.
>>
>> Thank you a lot for taking care about this!
>> I know, it was not easy to sort out the regressions observed in testing.
>>
>> Could you, please, explain the issue and fix to reviewers?
>>
>> Thanks,
>> Serguei
>>
>>
>> On 5/23/18 12:33, Alex Menkov wrote:
>>> Hi all,
>>>
>>> Please take a look at a fix for
>>> https://bugs.openjdk.java.net/browse/JDK-8187289
>>>
>>> webrev: http://cr.openjdk.java.net/~amenkov/notifyFramePop/webrev/
>>>
>>> CSR for the issue: https://bugs.openjdk.java.net/browse/JDK-8191467
>>>
>>> --alex
>>

[prev in list] [next in list] [prev in thread] [next in thread]