Hi, > On 27 Feb 2015, at 11:06, Peter Levart <peter.lev...@gmail.com> wrote: > On 02/27/2015 10:27 AM, Joel Borggrén-Franck wrote: >>> On 26 feb 2015, at 22:35, Peter Levart <peter.lev...@gmail.com> >>> wrote: >>> On 02/26/2015 10:27 PM, Peter Levart wrote: >>> >>>> The m.setAccessible(true) for the methods is needed to access methods of >>>> non-public annotations, right? This call could be moved to AnnotationType >>>> constructor as there it will be performed only once per Method object. >>>> >>> ...which will have the added benefit in that it will guarantee that only >>> one MethodAccessor object per Method will ever be constructed instead of >>> two… >>> >>> >> I don’t see this. setAccessible sets override in AccessibleObject, I don’t >> see a new MethodAccessor being generated here. > > My fault! I was mistakenly thinking of Field objects in the context of > setAccessible(boolean). If you use a Field object in two modes it will create > and retain two different FieldAccessors (because they are different > implementations in case field is final). > >> But I agree with you, and setting it as accessible in the AnnotationType >> constructor should arguably be more secure since then we know it isn’t >> shared since we just got our copy fresh from getDeclaredMethods(). > > That's a good point from maintainability perspective, yes, if someone some > time decides to "optimize" the AnnotationType. I think it would be nice if > AnnotationType documents that members() method returns Method objects that > are pre-conditioned with setAccessible(true) and that users should not change > this flag. >
I don’t want to do this in AnnotationType without a bigger overhaul that may be slightly incompatible and therefor should be 9 only. I do want to backport this fix to 8 however, so here is an alternative solution that should be safe and correct at the cost of extra allocation in the path for custom implementations of annotations (that should be rare). New webrev: http://cr.openjdk.java.net/~jfranck/8073056/webrev.02/ cheers /Joel
