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

[David Holmes]

On 24/12/2016 12:35 AM, George Triantafillou wrote:
> Hi David,
>
> On 12/22/2016 9:14 PM, David Holmes wrote:
>> 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!
>
> You'll need to run the latest jtreg to get all the latest jigsaw goodness:
>
> http://jre.us.oracle.com/java/re/jtreg/4.2/nightly/bundles/jtreg_bin-4.2-fcs-bin-b05_21_dec_2016.zip

Sounds more like I get a bug if I use that version. :) So is jtreg 
adding its own --add-opens and/or --add-exports based on the @module tags?

David
-----

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