RFR 8153629: Need to cover JVMTI's GetOwnedMonitorStackDepthInfo function

[Chris Plummer]

Hi Daniil,

Have you run with -Xcomp. I'm concerned that inlining will cause the 
test to fail because stack depths will not be as expected. Also, if you 
find the owned monitor, but the depth is not as expected, I think you 
are better off printing an error message right away rather than the 
somewhat misleading "FAIL: invalid number of %s monitors" message later on.

thanks,

Chris

On 1/16/18 6:31 PM, Daniil Titov wrote:
> Hello,
>
> Please review  an updated fix that makes the test more extensive and includes suggested changes.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8153629
> Webrev: http://cr.openjdk.java.net/~dtitov/8153629/webrev.02/
>
> Thank you!
>
> Best regards,
> Daniil
>
> On 1/12/18, 2:26 PM, "serguei.spitsyn at oracle.com" <serguei.spitsyn at oracle.com> wrote:
>
>      Hi Daniil,
>      
>      It is pretty good in general.
>      Thank you for taking care about it!
>      
>      Some comments though.
>      
>      The test case is too trivial.
>      I'd suggest to extend it to have at least a couple of locks in the
>      returned array.
>      One way to do it would be to add a instance synchronized method and
>      invoke it from the synchronized statement of the tested Thread.
>      Then the verifyOwnedMonitors() can be invoked from this method.
>      
>      A couple of comments on the native agent.
>      
>      72         // JNI_OnLoad has not been called yet, so can't possibly be an instance of TEST_CLASS.
>      
>      Could you, please, rewrite this comment?
>      Maybe just tell that there probably was an error in loading the TEST_CLASS.
>      What about moving the FindClass(env, TEST_CLASS) to the
>      verifyOwnedMonitors() function?
>      It will make the testClass variable local.
>      
>        200                 fprintf(stderr, "VerifyOwnedMonitors: FAIL: stackDepthInfo[0].stack_depth should be 1.\n");
>      
>        207         fprintf(stderr, "VerifyOwnedMonitors: FAIL: monitorCount should be 1.\n");
>      
>      
>      It'd better to rephrase the messages above to tell about actual values
>      vs expected.
>      It normally simplifies the analysis of failures as there is no need to find
>      what values were printed before and that they are exactly what needed
>      for comparison.
>      
>      Thanks,
>      Serguei
>      
>      
>      
>      On 1/11/18 17:45, Daniil Titov wrote:
>      > Hello,
>      >
>      > Please review the following fix that adds a jtreg test for GetOwnedMonitorStackDepthInfo JVMTI function.
>      >
>      > Bug: https://bugs.openjdk.java.net/browse/JDK-8153629
>      > Webrev: http://cr.openjdk.java.net/~dtitov/8153629/webrev.00
>      >
>      >
>      > The tests ran successfully with Mach5.
>      >
>      > Best regards,
>      > Daniil
>      >
>      >
>      
>      
>      
>      
>
>