RFR: 8214545: sun/management/jmxremote/bootstrap tests hang in revokeall.exe on Windows

[Boris Ulasevich]

The change is good. Thank you!

regards,
Boris

On 20.05.2019 3:43, David Holmes wrote:
> Hi Daniil,
> 
> cc: Boris and Erik J.
> 
> On 20/05/2019 7:12 am, Daniil Titov wrote:
>> Please review the change that fixes the failure of 
>> sun/management/jmxremote/bootstrap JMX tests on Windows platform.  
>> While running, these tests invoke revokeall.exe utility and this 
>> utility hangs.
>>
>> The problem here is that invokeall.exe goes into an endless loop  
>> while iterating over Access Control Entries (ACE) for a given file if 
>> it encounters at least one ACE with the type different from 
>> ACCESS_ALLOWED_ACE_TYPE.
>>
>> The change fixes this problem.  It also removes revokeall.exe binary 
>> from the repository and changes the makefile  to get it built instead.
>>
>> Tier1, tier2, tier3, jdk_svc, and sun/management/jmxremote/bootstrap  
>> tests succeeded  in Mach5.
>>
>> Webrev: http://cr.openjdk.java.net/~dtitov/8214545
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8214545
> 
> I knew this seemed very familiar ... Boris had a fix for this a few 
> weeks ago under JDK-8220581. Similar but not identical to yours - see 
> below. Though getting rid of the exe from the repo is a good idea 
> (thanks Erik!).
> 
> A few comments
> 
> test/jdk/sun/management/jmxremote/bootstrap/GeneratePropertyPassword.sh
> 
> Pre-existing:
> 
> !         REVOKEALL="$TESTNATIVEPATH/revokeall.exe"
>            if [ ! -f "$REVOKEALL" ] ; then
> 
> I would expect a -x test not -f.
> 
> ---
> 
> test/jdk/sun/management/windows/README
> 
> The first copyright year should be 2004.
> 
>    25 This directory contains the source and the binary version
> 
> Delete "and the binary version".
> 
> ---
> 
> test/jdk/sun/management/windows/exerevokeall.c
> 
> Pre-existing:
> 
>   31  * file - suitable for NT/2000/XP only.
> 
> Please delete everything after "file".
> 
> 
>   355             i++;
>   356             count--;
> 
> The count-- is obvious as it is the loop counter, but it is far from 
> clear to me that i++ is correct. I don't fully understand the logic but 
> i is only incremented under very specific conditions. If you rewrote the 
> code to avoid the use of the continue then i would not be modified 
> except where it currently is.
> 
> Thanks,
> David
> -----
> 
>> Thanks!
>> --Daniil
>>
>>