RFR (M): 8231595 [TEST] develop a test case for SuspendThreadList including current thread

[serguei.spitsyn at oracle.com]

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.spitsyn at 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