RFR 8153629: Need to cover JVMTI's GetOwnedMonitorStackDepthInfo function

[Chris Plummer]

Hi Serguei,

On 1/12/18 2:25 PM, 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.
This was copied from my comment in GetOwnedMonitorInfoTest, which I 
assume this test was based on.
> What about moving the FindClass(env, TEST_CLASS) to the 
> verifyOwnedMonitors() function?
> It will make the testClass variable local.
>
Also from GetOwnedMonitorInfoTest. This is code I reworked in that test 
recently to fix 8191229. These two tests should be kept consistent.

Chris
>  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
>>
>>
>