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

[Roger Riggs]

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/

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