Hi,

I looked over the changes and also my comments from when I reviewed for Serguei, and everything looks fine. I assume testing has been done on solaris and there was no need for the -lc there.

thanks,

Chris

On 5/29/18 1:59 PM, serguei.spit...@oracle.com wrote:
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.spit...@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



Reply via email to