RFR 6642881: Improve performance of Class.getClassLoader()

[Coleen Phillimore]

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.

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
>>