Review request for 7198429: need checked categorization of caller-sensitive methods in the JDK

[Peter Levart]

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-peter_2013_01_16_13_44-b00
# VM: OpenJDK 64-Bit Server VM 25.0-b14 (mixed mode)
# OS: Linux 3.7.9-104.fc17.x86_64 (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