Review request for 7198429: need checked categorization of caller-sensitive methods in the JDK (original) (raw)
Mandy Chung mandy.chung at oracle.com
Wed Apr 3 15:56:17 UTC 2013
- Previous message: Review request for 7198429: need checked categorization of caller-sensitive methods in the JDK
- Next message: RFR: JDK-8011187 : http://cr.openjdk.java.net/~mduigou/JDK-8011187/0/webrev/
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Peter,
Thanks. I misread your example and missed the fact that SM2 extends SM1. After seeing John's reply, I reread the example and realized the correctness issue you try to point out.
I'll revise it and send out a new webrev.
Mandy
On 4/3/2013 12:39 AM, Peter Levart wrote:
On 04/03/2013 12:25 AM, Mandy Chung wrote:
On 4/2/13 3:00 PM, Peter Levart wrote:
Hi Mandy,
There could be: public class SM1 extends SecurityManager { @Override public void checkMemberAccess(Class<?> clazz, int which) {... and: public class SM2 extends SM1 { ... // no checkMemberAccess override now if if you take SM2.class.getDeclaredMethod("checkMemberAccess", ...) it will throw NoSuchMethodException, but the method is overriden in SM1. So I think it's better to use what John suggested (although not using getDeclaredMethod, but getMethod instead): smgr.getClass().getMethod("checkMemberAccess", ...).getDeclaringClass() == SecurityManager.class Are you concerned the overhead of an exception thrown that we should avoid? Hi Mandy, No, I was not thinking of performance. Just the correctness. Class.getDeclaredMethod(name, ...) only considers methods "declared" by the class (not inherited from a superclass - directy or indirectly). But you could have, like in example above, a hierarchy: SM2 extends SM1 extends SecurityManager where SM1 overrides the method in SecurityManager and SM2 doesn't. Now if you try SM2.class.getDeclaredMethod(), you will get NoSuchMethodException, but that does not mean the method is not overriden - it is in class SM1. You could use getDeclaredMethod on the class and all superclasses until reaching SecurityManager to find out if the method is overriden, but it's fortunate that the method is public and so Class.getMethod(name, ...) can be used which does exactly that and already considers inheritance and overriding and returns the most specific method in the hierarchy. Explicit search using getDeclaredMethod() could skip searching the method in SecurityManager class though, but: - negative answer via exception is slow - getMethod() only searches among public methods (which are separately cached in special array) and can therefore be faster if there is relatively large share of non-public methods declared in the class being searched. Here's a quick micro-benchmark for the common case (only one level of subclassing the SecurityManager) and both variants (the method is overriden in SecurityManager subclass or not): ############################################################## # Java: 1.8.0-internal-peter201301161344-b00 # VM: OpenJDK 64-Bit Server VM 25.0-b14 (mixed mode) # OS: Linux 3.7.9-104.fc17.x8664 (amd64) # CPUs: 8 (virtual) # #------------------------------------------------------------- # GetDeclaredMethodOverridenSearch: run duration: 3,000 ms # # Warm up: # 1 threads, Tavg = 327.59 ns/op (σ = 0.00 ns/op) [ 327.59] # 1 threads, Tavg = 326.06 ns/op (σ = 0.00 ns/op) [ 326.06] # Measure: 1 threads, Tavg = 325.18 ns/op (σ = 0.00 ns/op) [ 325.18] # #------------------------------------------------------------- # GetPublicMethodOverridenSearch: run duration: 3,000 ms # # Warm up: # 1 threads, Tavg = 325.69 ns/op (σ = 0.00 ns/op) [ 325.69] # 1 threads, Tavg = 329.67 ns/op (σ = 0.00 ns/op) [ 329.67] # Measure: 1 threads, Tavg = 325.88 ns/op (σ = 0.00 ns/op) [ 325.88] # #------------------------------------------------------------- # GetDeclaredMethodNotOverridenSearch: run duration: 3,000 ms # # Warm up: # 1 threads, Tavg = 1,405.84 ns/op (σ = 0.00 ns/op) [ 1,405.84] # 1 threads, Tavg = 1,371.14 ns/op (σ = 0.00 ns/op) [ 1,371.14] # Measure: 1 threads, Tavg = 1,358.02 ns/op (σ = 0.00 ns/op) [ 1,358.02] # #------------------------------------------------------------- # GetPublicMethodNotOverridenSearch: run duration: 3,000 ms # # Warm up: # 1 threads, Tavg = 557.50 ns/op (σ = 0.00 ns/op) [ 557.50] # 1 threads, Tavg = 553.05 ns/op (σ = 0.00 ns/op) [ 553.05] # Measure: 1 threads, Tavg = 554.59 ns/op (σ = 0.00 ns/op) [ 554.59] # #------------------------------------------------------------- # END. ############################################################## Here's the source of the test: public class MethodSearchTest extends TestRunner { static final Class[] paramTypes = {Class.class, int.class}; static class SMOverriden extends SecurityManager { @Override public void checkMemberAccess(Class<?> clazz, int which) { } } static class SMNotOverriden extends SecurityManager { } public static class GetDeclaredMethodOverridenSearch extends Test { @Override protected void doLoop(Loop loop, DevNull devNull1, DevNull devNull2, DevNull devNull3, DevNull devNull4, DevNull devNull5) { while (loop.nextIteration()) { try { SMOverriden.class.getDeclaredMethod("checkMemberAccess", paramTypes); devNull1.yield(true); } catch (NoSuchMethodException e) { devNull1.yield(false); } } } } public static class GetPublicMethodOverridenSearch extends Test { @Override protected void doLoop(Loop loop, DevNull devNull1, DevNull devNull2, DevNull devNull3, DevNull devNull4, DevNull devNull5) { while (loop.nextIteration()) { try { devNull1.yield(SMOverriden.class.getMethod("checkMemberAccess", paramTypes).getDeclaringClass() != SecurityManager.class); } catch (NoSuchMethodException e) { throw new Error(e); } } } } public static class GetDeclaredMethodNotOverridenSearch extends Test { @Override protected void doLoop(Loop loop, DevNull devNull1, DevNull devNull2, DevNull devNull3, DevNull devNull4, DevNull devNull5) { while (loop.nextIteration()) { try { SMNotOverriden.class.getDeclaredMethod("checkMemberAccess", paramTypes); devNull1.yield(true); } catch (NoSuchMethodException e) { devNull1.yield(false); } } } } public static class GetPublicMethodNotOverridenSearch extends Test { @Override protected void doLoop(Loop loop, DevNull devNull1, DevNull devNull2, DevNull devNull3, DevNull devNull4, DevNull devNull5) { while (loop.nextIteration()) { try { devNull1.yield(SMNotOverriden.class.getMethod("checkMemberAccess", paramTypes).getDeclaringClass() != SecurityManager.class); } catch (NoSuchMethodException e) { throw new Error(e); } } } } public static void main(String[] args) throws Exception { startTests(); doTest(GetDeclaredMethodOverridenSearch.class, 3000L, 1, 1, 1); doTest(GetPublicMethodOverridenSearch.class, 3000L, 1, 1, 1); doTest(GetDeclaredMethodNotOverridenSearch.class, 3000L, 1, 1, 1); doTest(GetPublicMethodNotOverridenSearch.class, 3000L, 1, 1, 1); endTests(); } } Regards, Peter Mandy
- Previous message: Review request for 7198429: need checked categorization of caller-sensitive methods in the JDK
- Next message: RFR: JDK-8011187 : http://cr.openjdk.java.net/~mduigou/JDK-8011187/0/webrev/
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]