RFR(XS): JDK-8171318 - serviceability/sa/jmap-hprof/JMapHProfLargeHeapTest.java fails latest Jigsaw integration

[David Holmes]

Hi George,

Okay this is very perplexing. The test does not fail for me in its 
current form!

  > jtreg -a -v -testjdk:../../build/linux-x64-debug/images/jdk 
./serviceability/sa/jmap-hprof/JMapHProfLargeHeapTest.java
runner starting test: 
serviceability/sa/jmap-hprof/JMapHProfLargeHeapTest.java
runner finished test: 
serviceability/sa/jmap-hprof/JMapHProfLargeHeapTest.java
Passed. Execution successful
Test results: passed: 1

I am using jtreg, version 4.2 fcs b04 and have built VM from current 
jdk9/hs sources and jdk9/dev!

David
-----

On 23/12/2016 12:43 AM, George Triantafillou wrote:
> On 12/21/2016 7:44 PM, David Holmes wrote:
>
>> Hi George,
>>
>> On 22/12/2016 7:36 AM, George Triantafillou wrote:
>>> Hi David,
>>>
>>> New webrev: http://cr.openjdk.java.net/~gtriantafill/8171318/webrev.01/
>>> <http://cr.openjdk.java.net/%7Egtriantafill/8171318/webrev.01/>
>>
>> Using ProcessTools is much cleaner. This looks good.
> Thanks David.
>>
>> But I still could not get this test to fail locally. :)
> Here's my command line:
>
> $JAVA_HOME/bin/java -Djavatest.maxOutputSize=4000000 -jar
> ~/tools/jtreg_bin-4.2-fcs-bin-b05_21_dec_2016/jtreg/lib/jtreg.jar
> JMapHProfLargeHeapTest.java
>
> Timeout error:
>
> execStatus=Error. Program
> `/scratch/gctriant/projects/jdk9/jdk9/build/linux-x64/images/jdk/bin/java'
> timed out (timeout set to 120000ms, elapsed time including timeout
> handling was 121528ms).
>
> Actual error:
>
> Exception in thread "main"
> java.lang.reflect.InaccessibleObjectException: Unable to make field
> private final sun.management.VMManagement sun.management.RuntimeImpl.jvm
> accessible: module java.management does not "opens sun.management" to
> unnamed module @32709393
>         at
> java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:207)
>
>         at
> java.base/java.lang.reflect.Field.checkCanSetAccessible(Field.java:171)
>         at java.base/java.lang.reflect.Field.setAccessible(Field.java:165)
>         at
> JMapHProfLargeHeapProc.getProcessId(JMapHProfLargeHeapProc.java:59)
>         at JMapHProfLargeHeapProc.main(JMapHProfLargeHeapProc.java:41)
>
> -George
>>
>> Thanks,
>> David
>>
>>> Tested locally on Linux.
>>>
>>> More details inline:
>>>
>>> On 12/19/2016 11:13 PM, David Holmes wrote:
>>>
>>>> Hi George,
>>>>
>>>> On 20/12/2016 1:30 AM, George Triantafillou wrote:
>>>>> Please review this small change to fix test failures in
>>>>> JMapHProfLargeHeapTest.
>>>>>
>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8171318
>>>>> Open webrev: http://cr.openjdk.java.net/~gtriantafill/8171318/webrev/
>>>>> <http://cr.openjdk.java.net/%7Egtriantafill/8171318/webrev/>
>>>>
>>>> Why is this webrev based against jdk9/jdk9 instead of jdk9/hs?
>>> The webrev is based on jdk9/hs.  My clone script needs to updated.
>>>>
>>>>> JMapHProfLargeHeapTest.java: added add-opens to ProcessBuilder, added
>>>>> input check for largeHeapScanner.
>>>>
>>>> I can't make the existing test fail. I would have thought that
>>>> --add-exports subsumed the need to do --add-opens?
>>> Actually, --add-opens is required in addition to --add-exports to enable
>>> deep reflection.
>>>> I've been trying to find any doc to clarify that. But it seems to me
>>>> that this test as-is doesn't seem to fail since the most recent Jigsaw
>>>> refresh.
>>> With the latest jdk9, the test will always fail with a timeout, which is
>>> masked by the lack of an input check for largeHeapScanner. The actual
>>> error message follows:
>>>
>>> Exception java.lang.reflect.InaccessibleObjectException: Unable to make
>>> field private final sun.management.VMManagement
>>> sun.management.RuntimeImpl.jvm accessible: module java.management does
>>> not "opens sun.management" to unnamed module
>>>
>>> Christian pointed out in an offline discussion that a better solution
>>> uses jdk.test.lib.process.ProcessTools.getProcessId() in
>>> JMapHProfLargeHeapProc.java, which removes the call to setAccessible().
>>> Here's the new webrev:
>>>
>>> New webrev: http://cr.openjdk.java.net/~gtriantafill/8171318/webrev.01/
>>> <http://cr.openjdk.java.net/%7Egtriantafill/8171318/webrev.01/>
>>>
>>> Thanks.
>>>
>>> -George
>>>
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> Tested locally on Linux.
>>>>>
>>>>> Thanks.
>>>>>
>>>>> -George
>>>>>
>>>
>