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

[serguei.spitsyn at oracle.com]

On 12/21/16 15:33, George Triantafillou wrote:
> Hi Serguei,
>
> On 12/21/2016 5:59 PM, serguei.spitsyn at oracle.com wrote:
>> Hi George,
>>
>>
>> It looks good but I have a question.
>> Why the option --add-opens is not passed now?
>> Does it mean enabling deep reflection is not needed anymore?
> Deep reflection is no longer needed since the call to setAccessible() 
> in JMapHProfLargeHeapProc.java was removed.  It was replaced with 
> ProcessTools.getProcessId(), which uses 
> ProcessHandle.current().getPid() to get the current process ID.

Ok, thanks.
It was my guess. :)

Thanks, George!
Serguei


>
> -George
>
>>
>> Thanks,
>> Serguei
>>
>>
>>
>> On 12/21/16 13:36, 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/>
>>>
>>> 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
>>>>>
>>>
>>
>