RFR 8210300: runtime/MemberName/MemberNameLeak.java fails with RuntimeException

[Patricio Chilano]

Hi Dan,

On 9/10/18 11:51 AM, Daniel D. Daugherty wrote:
> On 9/10/18 9:57 AM, Patricio Chilano wrote:
>> Hi all,
>>
>> Could you review this small fix to tests CleanProtectionDomain.java 
>> and MemberNameLeak.java?
>> It adds flag UnlockDiagnosticVMOptions needed in product builds. It 
>> also calls System.gc() periodically to avoid relaying on a single 
>> call, and adds a short sleep between polls.
>>
>> Bug URL: https://bugs.openjdk.java.net/browse/JDK-8210300
>> Webrev URL: http://cr.openjdk.java.net/~pchilanomate/8210300.01/webrev
>
> test/hotspot/jtreg/runtime/Dictionary/CleanProtectionDomain.java
>     L78:       while(true) {
>     L81:         if(removedCountOrig != removedCount) break;
>         nit - need a space before '(' on each line
>
> test/hotspot/jtreg/runtime/MemberName/MemberNameLeak.java
>     L61:         while(true) {
>     L64:           if(removedCountOrig != removedCount) break;
>         nit - need a space before '(' on each line
>
> I'm curious: why change from a do-while loop to a "while (true)"
> loop with an if-statement break?
Seemed more natural with the Thread.sleep() addition, since it should go 
after checking if removedCount equals removedCountOrig. Otherwise I 
would had to add Thread.sleep() at the beginning of the do-while which 
logically didn't make sense, so that would have implied adding a 
System.gc(), a read to removedCount and a comparison once before the 
do-while which would have added more code. Alternatively I could have 
kept the do-while changing the condition to true and using the same 
if-statement break, but being all the same I prefer using a while(true) 
loop than a do-while(true)  : )
> Thumbs up! I don't need to see a new webrev for fixing the nits.
>
> Dan

Thanks for the review Dan!

Patricio

>> Thanks,
>> Patricio
>