Mandy, Paul, what do you think? cheers /Joel
> On 28 Mar 2015, at 11:24, Joel Borggrén-Franck <joel.fra...@oracle.com> wrote: > > 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
