RFR(XS): JDK-8171318 - serviceability/sa/jmap-hprof/JMapHProfLargeHeapTest.java fails latest Jigsaw integration
George Triantafillou
george.triantafillou at oracle.com
Thu Dec 22 14:21:16 UTC 2016
Thanks Serguei.
-George
On 12/21/2016 6:35 PM, serguei.spitsyn at oracle.com wrote:
> 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
>>>>>>
>>>>
>>>
>>
>
More information about the hotspot-runtime-dev
mailing list