Fix proposal: JDK-8219378 NPE in ReflectionFactory.newMethodAccessor when langReflectAccess not initialized

[Mandy Chung]

On 2/26/19 8:56 AM, Roger Riggs wrote:
> Hi Andrew,
>
> I've got the changeset you sent earlier and have tested it as well.
>
> I'll push it today unless there any objections.
>
> Webrev: 
> http://cr.openjdk.java.net/~rriggs/webrev-reflectionfactory-8219378-2/
>

+1

Mandy

> Roger
>
>
> On 02/26/2019 11:46 AM, Andrew Leonard wrote:
>> Hi Mandy,
>> I think your last proposal sounds a good plan, push 8219378 to fix 
>> the immediate issue and create a new change to refactor the clinit 
>> location...
>> As I am not a "Committer" (yet! working on it!) I can't use the 
>> Submit repo.
>> I have built your patch and run a bunch of tests locally and it works 
>> great. I have also tested it successfully with J9. So I am happy with 
>> your change.
>>
>> If Roger & yourself are happy with 8219378, can we Submit-repo it for 
>> final test and get that one merged?
>> Many thanks
>> Andrew
>>
>> Andrew Leonard
>> Java Runtimes Development
>> IBM Hursley
>> IBM United Kingdom Ltd
>> Phone internal: 245913, external: 01962 815913
>> internet email: andrew_m_leonard at uk.ibm.com
>>
>>
>>
>>
>> From: Mandy Chung <mandy.chung at oracle.com>
>> To: Andrew Leonard <andrew_m_leonard at uk.ibm.com>
>> Cc: core-libs-dev at openjdk.java.net, Roger Riggs <Roger.Riggs at oracle.com>
>> Date: 26/02/2019 00:16
>> Subject: Re: Fix proposal: JDK-8219378 NPE in 
>> ReflectionFactory.newMethodAccessor when langReflectAccess not 
>> initialized
>> ------------------------------------------------------------------------
>>
>>
>>
>>
>>
>> On 2/25/19 5:12 AM, Andrew Leonard wrote:
>> > Hi Mandy,
>> > I must admit I don't completely follow the logic of the existing
>> > Modifier init of langReflectAccess, the comment indicates a "protocol
>> > between java.lang and java.lang.reflect".
>>
>> That sets up the shared secret for ReflectionFactory to access
>> non-public members in java.lang.reflect.
>>
>> > I can try moving the clinit
>> > code from Modifier to AccessibleObject, but I question is there some
>> > reason it is there? Would we be sure in moving it we are not missing
>> > something?
>>
>> ReflectionFactory is the internal support for reflection.
>> The methods that access LangReflectAccess shared secrets
>> should have a Method, Field or Constructor in hand.  The
>> ReflectionFactory::newField and newMethod that don't take
>> Field/Method parameter are unused (I suspect they were
>> used by the VM native reflection implementation previously.
>> That led me to suggest to move setLangReflectAccess to
>> AccessibleObject.
>>
>> AccessibleObject is initialized very early during startup
>> by the VM.  My proposed fix would change the list of
>> classes loaded during early startup but it would need to
>> look at closely.
>>
>> Having a second thought, my proposed fix can be a follow-on
>> clean up. I'm okay with your point fix that resolves JDK-8219378.
>> I will file a JBS issue for the follow-on clean up.  What do
>> you think?
>>
>> thanks
>> Mandy
>>
>>
>>
>>
>> Unless stated otherwise above:
>> IBM United Kingdom Limited - Registered in England and Wales with 
>> number 741598.
>> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire 
>> PO6 3AU
>