Hi Serguei,
On 1/12/18 2:25 PM, serguei.spit...@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
