Hi, > On 25 feb 2015, at 16:32, Peter Levart <[email protected]> wrote: > On 02/25/2015 04:10 PM, Joel Borggrén-Franck wrote: >> Hi Peter, >> >> >>> On 25 feb 2015, at 15:48, Peter Levart <[email protected]> >>> wrote: >>> >>> >>> On 02/25/2015 03:26 PM, Peter Levart wrote: >>> >>>> Ah, never mind. I missed the explicit access check that >>>> getEnclosingMethod() already performs on it's own before calling >>>> getDeclaredMethods(). But the check is for the same permission and could >>>> be performed implicitly by getDeclaredMethods() if the caller of >>>> getEnclosingMethod() was passed to it. Just an optimization opportunity >>>> therefore. >>>> >>>> Peter >>>> >>> Or, even a better optimization: >>> >>> - leave explicit check in getEnclosingMethod() as is >>> - call privateGetDeclaredMethods(false) instead to iterate through methods >>> - return a defensive copy of just the matching Method if found >>> >> I did consider variations of this, but there is always a risk that someone >> someday forgets to copy methods, or uses a potential innerGetDeclaredMethods >> without making the proper checking beforehand. So I went for the IMO more >> maintainable solution. I suppose if this turns out to be a drag on >> performance we might be able to skip the doPrivileged if there is no >> SecurityManager set. > > getDeclaredMethod(name, types) is simmilar. It does explicit permissions > check (as does getEnclosingMethod), it calls > privateGetDeclaredMethods(false), searches for matching method and copies it > before returning. That's very similar to getEnclosingMethod(), just the Class > target and search criteria are different. > > IMO, a comment at permission check and at defensive copying of the Method > object is enough for a maintainer to watch out what (s)he's doing, but I > guess this method is not so frequently used as getDeclaredMethod() to be > important that it is optimal.
My argument here is that every time we add uses of code that requires explicit permission checks, there is a risk that check will be forgotten someday. As you write, my assumption is that this code isn’t in the hot path of performance critical code. If real world use cases show up where this is to slow, we can revisit. cheers /Joel
