dorkbox/Network
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fixed RMI bug with overriding methods w/ connection as first parameter

Browse Source
This commit is contained in:
nathan 2015-12-18 03:38:12 +01:00
parent 1d3531afc0
commit 80b358f8aa
4 changed files with 71 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Dorkbox-Network/src/dorkbox/network/rmi/AsmCachedMethod.java
View File

@ -48,7 +48,7 @@ class AsmCachedMethod extends CachedMethod {
public
Object invoke(final Connection connection, Object target, Object[] args) throws IllegalAccessException, InvocationTargetException {
try {
if (origMethod == null) {
if (origMethod == method) {
return this.methodAccess.invoke(target, this.methodAccessIndex, args);
}
else {

69
Dorkbox-Network/src/dorkbox/network/rmi/CachedMethod.java
View File

@ -41,6 +41,8 @@ import com.esotericsoftware.reflectasm.MethodAccess;
import dorkbox.network.connection.Connection;
import dorkbox.network.connection.EndPoint;
import dorkbox.util.ClassHelper;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
@ -50,10 +52,13 @@ import java.util.concurrent.ConcurrentHashMap;
public
class CachedMethod {
private static final Logger logger = LoggerFactory.getLogger(CachedMethod.class);
// not concurrent because they are setup during system initialization
public static final Map<Class<?>, Class<?>> overriddenMethods = new HashMap<Class<?>, Class<?>>();
public static final Map<Class<?>, Class<?>> overriddenReverseMethods = new HashMap<Class<?>, Class<?>>();
// the purpose of the method cache, is to accelerate looking up methods for specific class
private static final Map<Class<?>, CachedMethod[]> methodCache = new ConcurrentHashMap<Class<?>, CachedMethod[]>(EndPoint.DEFAULT_THREAD_POOL_SIZE);
// type will be likely be the interface
@ -81,8 +86,12 @@ class CachedMethod {
// This MUST hold valid for both remote and local connection types.
// To facilitate this functionality, for methods with the same name, the "overriding" method is the one that inherits the Connection
// interface as the first parameter.
// interface as the first parameter, and .registerRemote(ifaceClass, implClass) must be called.
Map<Method, Method> overriddenMethods = getOverriddenMethods(type, methods);
final boolean hasOverriddenMethods = !overriddenMethods.isEmpty();
MethodAccess methodAccess = null;
@ -99,30 +108,36 @@ class CachedMethod {
Class<?>[] parameterTypes = method.getParameterTypes();
Class<?>[] asmParameterTypes = parameterTypes;
Method overriddenMethod = overriddenMethods.remove(method);
boolean overridden = overriddenMethod != null;
if (overridden) {
// we can override the details of this method BECAUSE (and only because) our kryo registration override will return
// the correct object for this overridden method to be called on.
method = overriddenMethod;
if (hasOverriddenMethods) {
Method overriddenMethod = overriddenMethods.remove(method);
boolean overridden = overriddenMethod != null;
if (overridden) {
// we can override the details of this method BECAUSE (and only because) our kryo registration override will return
// the correct object for this overridden method to be called on.
method = overriddenMethod;
Class<?> overrideType = method.getDeclaringClass();
Class<?> overrideType = method.getDeclaringClass();
if (kryo.getAsmEnabled() && !Util.isAndroid && Modifier.isPublic(overrideType.getModifiers())) {
localMethodAccess = MethodAccess.get(overrideType);
asmParameterTypes = method.getParameterTypes();
if (kryo.getAsmEnabled() && !Util.isAndroid && Modifier.isPublic(overrideType.getModifiers())) {
localMethodAccess = MethodAccess.get(overrideType);
asmParameterTypes = method.getParameterTypes();
}
}
}
CachedMethod cachedMethod = null;
if (localMethodAccess != null) {
try {
final int index = localMethodAccess.getIndex(method.getName(), asmParameterTypes);
AsmCachedMethod asmCachedMethod = new AsmCachedMethod();
asmCachedMethod.methodAccessIndex = localMethodAccess.getIndex(method.getName(), asmParameterTypes);
asmCachedMethod.methodAccessIndex = index;
asmCachedMethod.methodAccess = localMethodAccess;
cachedMethod = asmCachedMethod;
} catch (RuntimeException ignored) {
ignored.printStackTrace();
} catch (RuntimeException e) {
if (logger.isTraceEnabled()) {
logger.trace("Unable to use ReflectAsm for {}:{}", method.getDeclaringClass(), method.getName(), e);
}
}
}
@ -162,8 +177,8 @@ class CachedMethod {
for (Method origMethod : origMethods) {
String name = origMethod.getName();
Class<?>[] types = origMethod.getParameterTypes();
int modLength = types.length + 1;
Class<?>[] origTypes = origMethod.getParameterTypes();
int origLength = origTypes.length + 1;
METHOD_CHECK:
for (Method implMethod : implMethods) {
@ -171,18 +186,24 @@ class CachedMethod {
Class<?>[] checkTypes = implMethod.getParameterTypes();
int checkLength = checkTypes.length;
if (modLength != checkLength || !(name.equals(checkName))) {
if (origLength != checkLength || !(name.equals(checkName))) {
continue;
}
// checkLength > 0
Class<?> checkType = checkTypes[0];
if (ClassHelper.hasInterface(dorkbox.network.connection.Connection.class, checkType)) {
Class<?> shouldBeConnectionType = checkTypes[0];
if (ClassHelper.hasInterface(dorkbox.network.connection.Connection.class, shouldBeConnectionType)) {
// now we check to see if our "check" method is equal to our "cached" method + Connection
for (int k = 1; k < checkLength; k++) {
if (types[k - 1] == checkTypes[k]) {
overrideMap.put(origMethod, implMethod);
break METHOD_CHECK;
if (checkLength == 1) {
overrideMap.put(origMethod, implMethod);
break;
}
else {
for (int k = 1; k < checkLength; k++) {
if (origTypes[k - 1] == checkTypes[k]) {
overrideMap.put(origMethod, implMethod);
break METHOD_CHECK;
}
}
}
}
@ -284,7 +305,7 @@ class CachedMethod {
public
Object invoke(final Connection connection, Object target, Object[] args) throws IllegalAccessException, InvocationTargetException {
// did we override our cached method?
if (origMethod == null) {
if (method == origMethod) {
return this.method.invoke(target, args);
}
else {

31
Dorkbox-Network/src/dorkbox/network/rmi/RemoteInvocationHandler.java
View File

@ -80,7 +80,7 @@ class RemoteInvocationHandler implements InvocationHandler {
private boolean udt;
private Byte lastResponseID;
private byte nextResponseId = 1;
private byte nextResponseId = (byte) 1;
public
RemoteInvocationHandler(final Connection connection, final int objectID) {
@ -126,7 +126,7 @@ class RemoteInvocationHandler implements InvocationHandler {
.add(this.responseListener);
}
@SuppressWarnings({"AutoUnboxing", "AutoBoxing"})
@SuppressWarnings({"AutoUnboxing", "AutoBoxing", "NumericCastThatLosesPrecision"})
@Override
public
Object invoke(final Object proxy, final Method method, final Object[] args) throws Exception {
@ -196,7 +196,7 @@ class RemoteInvocationHandler implements InvocationHandler {
final Logger logger1 = RemoteInvocationHandler.logger;
EndPoint<Connection> endPoint = this.connection.getEndPoint();
EndPoint endPoint = this.connection.getEndPoint();
final CryptoSerializationManager serializationManager = endPoint.getSerialization();
InvokeMethod invokeMethod = new InvokeMethod();
@ -224,17 +224,20 @@ class RemoteInvocationHandler implements InvocationHandler {
// In situations where we want to pass in the Connection (to an RMI method), we have to be able to override method A, with method B.
// This is to support calling RMI methods from an interface (that does pass the connection reference) to
// an implementation, that DOES pass the connection reference. The remote side (that initiates the RMI calls), MUST use
// the interface, and the implementation may override the method, so that we add the connection as the first in
// an implType, that DOES pass the connection reference. The remote side (that initiates the RMI calls), MUST use
// the interface, and the implType may override the method, so that we add the connection as the first in
// the list of parameters.
//
// for example:
// Interface: foo(String x)
// Impl: foo(Connection c, String x)
//
// The implementation (if it exists, with the same name, and with the same signature+connection) will be called from the interface.
// The implType (if it exists, with the same name, and with the same signature+connection) will be called from the interface.
// This MUST hold valid for both remote and local connection types.
// To facilitate this functionality, for methods with the same name, the "overriding" method is the one that inherits the Connection
// interface as the first parameter, and .registerRemote(ifaceClass, implClass) must be called.
if (checkMethod.equals(method)) {
invokeMethod.cachedMethod = cachedMethod;
break;
@ -249,28 +252,28 @@ class RemoteInvocationHandler implements InvocationHandler {
// An invocation doesn't need a response is if it's async and no return values or exceptions are wanted back.
boolean needsResponse = !this.udp && (this.transmitReturnValue || this.transmitExceptions || !this.nonBlocking);
byte responseID = 0;
byte responseID = (byte) 0;
if (needsResponse) {
synchronized (this) {
// Increment the response counter and put it into the low bits of the responseID.
responseID = this.nextResponseId++;
if (this.nextResponseId > RmiBridge.responseIdMask) {
this.nextResponseId = 1;
this.nextResponseId = (byte) 1;
}
this.pendingResponses[responseID] = true;
}
// Pack other data into the high bits.
byte responseData = responseID;
if (this.transmitReturnValue) {
responseData |= RmiBridge.returnValueMask;
responseData |= (byte) RmiBridge.returnValueMask;
}
if (this.transmitExceptions) {
responseData |= RmiBridge.returnExceptionMask;
responseData |= (byte) RmiBridge.returnExceptionMask;
}
invokeMethod.responseData = responseData;
}
else {
invokeMethod.responseData = 0; // A response data of 0 means to not respond.
invokeMethod.responseData = (byte) 0; // A response data of 0 means to not respond.
}
if (this.udp) {
@ -314,13 +317,13 @@ class RemoteInvocationHandler implements InvocationHandler {
return Boolean.FALSE;
}
if (returnType == float.class) {
return 0f;
return 0.0f;
}
if (returnType == char.class) {
return (char) 0;
}
if (returnType == long.class) {
return 0l;
return 0L;
}
if (returnType == short.class) {
return (short) 0;
@ -329,7 +332,7 @@ class RemoteInvocationHandler implements InvocationHandler {
return (byte) 0;
}
if (returnType == double.class) {
return 0d;
return 0.0d;
}
}
return null;

9
Dorkbox-Network/src/dorkbox/network/rmi/RmiBridge.java
View File

@ -70,11 +70,17 @@ import java.util.concurrent.locks.ReentrantReadWriteLock.WriteLock;
* the connection reference. The remote side (that initiates the RMI calls), MUST use the interface, and the implementation may override the
* method, so that we add the connection as the first in the list of parameters.
* <p/>
* for example: Interface: foo(String x) Impl: foo(Connection c, String x)
* for example:
* Interface: foo(String x)
* Impl: foo(Connection c, String x)
* <p/>
* The implementation (if it exists, with the same name, and with the same signature+connection) will be called from the interface. This
* MUST hold valid for both remote and local connection types.
*
* To facilitate this functionality, for methods with the same name, the "overriding" method is the one that inherits the Connection
* interface as the first parameter, and CachedMethod.registerOverridden(ifaceClass, implClass) must be called.
*
*
* @author Nathan Sweet <misc@n4te.com>, Nathan Robinson
*/
public
@ -227,6 +233,7 @@ class RmiBridge {
* @param connection
* The remote side of this connection requested the invocation.
*/
@SuppressWarnings("NumericCastThatLosesPrecision")
protected
void invoke(final Connection connection, final Object target, final InvokeMethod invokeMethod) throws IOException {
CachedMethod cachedMethod = invokeMethod.cachedMethod;