Hi Alex,
Thank you for reviewing this!
BTW, I forgot to say that Alex already did a pre-review of this,
and I've resolved many of his early comments/suggestions.
On 9/30/19 13:41, Alex Menkov wrote:
Name of the test doesn't look good (it's too general).
Maybe SuspendWithCurrentThread?
Good suggestion, thanks!
72 throw new RuntimeException("Man: unable to prepare
tested thread: " + threads[i]);
Do you mean "Main" (instead of "Man")?
Yes, it is a typo - nice catch!
In "run" method you don't actually need "boolean success"
109 success = checkSuspendedStatus(threads);
110 if (!success) {
111 throw new RuntimeException("Main: FAILED status
returned from checkTestedThreadsSuspended");
112 }
==>
if (!checkSuspendedStatus(threads)) {
throw new RuntimeException(...);
}
115 success = resumeTestedThreads();
116 if (!success) {
117 throw new RuntimeException("Main: FAILED status
returned from resumeTestedThreads");
118 }
==>
if (!resumeTestedThreads()) {
throw new RuntimeException(...);
}
This local was to make the code more readable.
But I can get rid of it if you think it is not important.
Looks like all native methods (except checkTestedThreadsSuspended) can
return only JNI_TRUE (on error they call jni->FatalError)
So they can be void (and don't need to test returned value)
Okay, I'll double check if we can do this simplification.
Thanks,
Serguei
--alex
On 09/27/2019 18:25, serguei.spit...@oracle.com wrote:
Please, review fix for test enhancement:
https://bugs.openjdk.java.net/browse/JDK-8231595
Webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8231595-jvmti-susp-tlist.1/
Summary:
New test is a coverage for the JVMTI bug:
https://bugs.openjdk.java.net/browse/JDK-8217762
SuspendThreadList won't work correctly if the current thread
is not last in the list
It provides a prove the bug JDK-8217762 does not exist
as the test is passed with the current implementation.
Thanks,
Serguei
