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

[Erik Joelsson]

Build changes look good.

/Erik

On 2019-05-19 17: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
>>
>>