RFR 6642881: Improve performance of Class.getClassLoader()

[Coleen Phillimore]

On 6/23/14, 9:36 PM, Mandy Chung wrote:
> Coleen,
>
> On 6/23/2014 4:45 PM, Coleen Phillimore wrote:
>>
>> Please review a change to the JDK code for adding classLoader field 
>> to the instances of java/lang/Class.  This change restricts 
>> reflection from changing access to the classLoader field.  In the 
>> spec, AccessibleObject.setAccessible() may throw SecurityException if 
>> the accessibility of an object may not be changed:
>>
>> http://docs.oracle.com/javase/7/docs/api/java/lang/reflect/AccessibleObject.html#setAccessible(boolean) 
>>
>>
>> Only AccessibleObject.java has changes from the previous version of 
>> this change.
>>
>
> This change looks reasonable.    As a side note:  Joel mentions about 
> the mechanism to hide class members from reflection.   I discussed 
> with Joel offline before he went on parental leave and suggest that we 
> should revisit the two mechanisms that both effectively disallow 
> access to private members in the future.

Thanks, Mandy.  Yes, let me know what you come up with and we can 
improve this.  Thank you for the help fixing this bug.

Coleen

>
> Mandy
>
>> open webrev at http://cr.openjdk.java.net/~coleenp/6642881_jdk_4/
>> bug link https://bugs.openjdk.java.net/browse/JDK-6642881
>>
>> Thanks,
>> Coleen
>>
>> On 6/19/14, 11:01 PM, David Holmes wrote:
>>> On 20/06/2014 6:53 AM, Joel Borggrén-Franck wrote:
>>>> Hi Mandy,
>>>>
>>>> On 19 jun 2014, at 22:34, Mandy Chung <mandy.chung at oracle.com> wrote:
>>>>
>>>>> On 6/19/14 12:34 PM, Joel Borggrén-Franck wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 19 jun 2014, at 20:46, Coleen Phillimore 
>>>>>> <coleen.phillimore at oracle.com> wrote:
>>>>>>> On 6/17/14, 12:38 AM, Joel Borggrén-Franck wrote:
>>>>>>>> Have you considered hiding the Class.classLoader field from 
>>>>>>>> reflection? I’m not sure it is necessary but I’m not to keen on 
>>>>>>>> the idea of people poking at this field with Unsafe (which 
>>>>>>>> should go away in 9 but …).
>>>>>>> I don't know how to hide the field from reflection. It's a 
>>>>>>> private final field so you need to get priviledges to make it 
>>>>>>> setAccessible.  If you mean injecting it on the JVM side, the 
>>>>>>> reason for this change is so that the JIT compilers can inline 
>>>>>>> this call and not have to call into the JVM to get the class 
>>>>>>> loader.
>>>>>>>
>>>>>> There is sun.reflect.Reflection.registerFieldsToFilter() that 
>>>>>> hides a field from being found using reflection. It might very 
>>>>>> well be the case that private and final is enough, I haven’t 
>>>>>> thought this through 100%. On the other hand, is there a reason 
>>>>>> to give users access through the field instead of having to use 
>>>>>> Class.getClassLoader()?
>>>>>>
>>>>> There are many getter methods that returns a private final field.
>>>>> I'm not sure if hiding the field is necessary nor a good precedence
>>>>> to set. Accessing a private field requires "accessDeclaredMember"
>>>>> permission although it's a different security check (vs 
>>>>> "getClassLoader"
>>>>> permission).  Arguably before this new classLoader field, one could
>>>>> call Class.getClassLoader0() via reflection to get a hold of class
>>>>> loader.
>>>>>
>>>>> Perhaps you are concerned that the "accessDeclaredMember" permission
>>>>> is too coarse-grained?  I think the security team is looking into
>>>>> the improvement in that regards.
>>>>
>>>> I think I’m a bit worried about two things, first as you wrote, 
>>>> “accessDeclaredMember” isn’t the same as “getClassLoader”, but 
>>>> since you could get the class loader through getClassLoader0() that 
>>>> shouldn’t be a new issue.
>>>>
>>>> The second thing is that IIRC there are ways to set a final field 
>>>> after initialization. I’m not sure we need to care about that 
>>>> either if you need Unsafe to do it.
>>>
>>> Normal reflection can set a final field if you can successfully call 
>>> setAccessible(true) on the Field object.
>>>
>>> David
>>> -----
>>>
>>>
>>>> cheers
>>>> /Joel
>>>>
>>
>