On 03/28/2015 11:24 AM, Joel Borggrén-Franck 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
Hi Joel,
If you really must add new methods to Method, ReflectAccess,
LangReflectAccess and ReflectionFactory then you could expose a method
called "invokeUnchecked" that would skip access checks, so you don't
have to copy the Method object just to get a one-time access to the
possibly inaccessible method. But this is good as it is, since the code
path is normally not used - it's there only for consistency's sake, right?
Regards, Peter
