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

[David Holmes]

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
> 
>