From 6c97567d04fae3c53424b031be4af9f1ad62b0bb Mon Sep 17 00:00:00 2001 From: nathan Date: Mon, 25 Sep 2017 23:15:32 +0200 Subject: [PATCH] RMI cleanup/polish --- .../CryptoSerializationManager.java | 49 ++++++----- src/dorkbox/network/rmi/ClassDefinitions.java | 81 ------------------- src/dorkbox/network/rmi/RmiProxyHandler.java | 3 - 3 files changed, 31 insertions(+), 102 deletions(-) delete mode 100644 src/dorkbox/network/rmi/ClassDefinitions.java diff --git a/src/dorkbox/network/connection/CryptoSerializationManager.java b/src/dorkbox/network/connection/CryptoSerializationManager.java index d95d84e8..98cbaee3 100644 --- a/src/dorkbox/network/connection/CryptoSerializationManager.java +++ b/src/dorkbox/network/connection/CryptoSerializationManager.java @@ -38,11 +38,11 @@ import com.esotericsoftware.kryo.io.Input; import com.esotericsoftware.kryo.io.Output; import com.esotericsoftware.kryo.serializers.CollectionSerializer; import com.esotericsoftware.kryo.serializers.FieldSerializer; +import com.esotericsoftware.kryo.util.IdentityMap; import com.esotericsoftware.kryo.util.IntMap; import com.esotericsoftware.kryo.util.MapReferenceResolver; import dorkbox.network.connection.ping.PingMessage; -import dorkbox.network.rmi.ClassDefinitions; import dorkbox.network.rmi.InvocationHandlerSerializer; import dorkbox.network.rmi.InvocationResultSerializer; import dorkbox.network.rmi.InvokeMethod; @@ -178,7 +178,8 @@ class CryptoSerializationManager implements dorkbox.network.util.CryptoSerializa // used to track which interface -> implementation, for use by RMI private final IntMap> rmiIdToImpl = new IntMap>(); private final IntMap> rmiIdToIface = new IntMap>(); - private final ClassDefinitions classDefinitions = new ClassDefinitions(); + private final IdentityMap, Class> rmiIfacetoImpl = new IdentityMap, Class>(); + private final IdentityMap, Class> rmiImplToIface = new IdentityMap, Class>(); /** * By default, the serialization manager will compress+encrypt data to connections with remote IPs, and only compress on the loopback IP @@ -272,6 +273,7 @@ class CryptoSerializationManager implements dorkbox.network.util.CryptoSerializa int id = kryo.register(remoteImplClass.implClass, remoteObjectSerializer).getId(); // sets up the RMI, so when we receive the iface class from the client, we know what impl to use + // if this is over-written, we don't care. rmiIdToImpl.put(id, remoteImplClass.implClass); rmiIdToIface.put(id, remoteImplClass.ifaceClass); } @@ -403,7 +405,15 @@ class CryptoSerializationManager implements dorkbox.network.util.CryptoSerializa classesToRegister.add(new RemoteImplClass(ifaceClass, implClass)); // this MUST BE UNIQUE otherwise unexpected things can happen. - classDefinitions.set(ifaceClass, implClass); + Class a = this.rmiIfacetoImpl.put(ifaceClass, implClass); + Class b = this.rmiImplToIface.put(implClass, ifaceClass); + + // this MUST BE UNIQUE otherwise unexpected things can happen. + if (a != null || b != null) { + throw new IllegalArgumentException("Unable to override interface (" + ifaceClass + ") and implementation (" + implClass + ") " + + "because they have already been overridden by something else. It is critical that they are" + + " both unique per JVM"); + } return this; } @@ -439,7 +449,9 @@ class CryptoSerializationManager implements dorkbox.network.util.CryptoSerializa public synchronized void finishInit() { initialized = true; - // initialize the kryo pool with at least 1 kryo instance. This ALSO makes sure that all of our class registration is done correctly + + // initialize the kryo pool with at least 1 kryo instance. This ALSO makes sure that all of our class registration is done + // correctly and (if not) we are are notified on the initial thread (instead of on the network udpate thread) kryoPool.put(takeKryo()); } @@ -461,6 +473,19 @@ class CryptoSerializationManager implements dorkbox.network.util.CryptoSerializa return rmiIdToIface.get(objectId); } + /** + * Gets the RMI implementation based on the specified ID (which is the ID for the registered interface) + * + * @param objectId ID of the registered interface, which will map to the corresponding implementation. + * + * @return the implementation for the interface, or null + */ + @Override + public + Class getRmiImpl(int objectId) { + return rmiIdToImpl.get(objectId); + } + /** * Gets the RMI implementation based on the specified interface * @@ -469,7 +494,7 @@ class CryptoSerializationManager implements dorkbox.network.util.CryptoSerializa @Override public Class getRmiImpl(Class iface) { - return classDefinitions.get(iface); + return rmiIfacetoImpl.get(iface); } /** @@ -480,19 +505,7 @@ class CryptoSerializationManager implements dorkbox.network.util.CryptoSerializa @Override public Class getRmiIface(Class implementation) { - return classDefinitions.getReverse(implementation); - } - - /** - * Gets the RMI implementation based on the specified ID (which is the ID for the registered interface) - * - * @param objectId ID of the registered interface, which will map to the corresponding implementation. - * @return the implementation for the interface, or null - */ - @Override - public - Class getRmiImpl(int objectId) { - return rmiIdToImpl.get(objectId); + return rmiImplToIface.get(implementation); } /** diff --git a/src/dorkbox/network/rmi/ClassDefinitions.java b/src/dorkbox/network/rmi/ClassDefinitions.java deleted file mode 100644 index f3e44c29..00000000 --- a/src/dorkbox/network/rmi/ClassDefinitions.java +++ /dev/null @@ -1,81 +0,0 @@ -/* - * Copyright 2016 dorkbox, llc - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package dorkbox.network.rmi; - -import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; - -import com.esotericsoftware.kryo.util.IdentityMap; - -/** - * Uses the "single writer principle" for fast access - */ -public -class ClassDefinitions { - // not concurrent because they are setup during system initialization - private volatile IdentityMap, Class> overriddenMethods = new IdentityMap, Class>(); - private volatile IdentityMap, Class> overriddenReverseMethods = new IdentityMap, Class>(); - - private static final AtomicReferenceFieldUpdater overriddenMethodsREF = - AtomicReferenceFieldUpdater.newUpdater(ClassDefinitions.class, - IdentityMap.class, - "overriddenMethods"); - - private static final AtomicReferenceFieldUpdater overriddenReverseMethodsREF = - AtomicReferenceFieldUpdater.newUpdater(ClassDefinitions.class, - IdentityMap.class, - "overriddenReverseMethods"); - - public - ClassDefinitions() { - } - - /** - * access a snapshot of the overridden methods (single-writer-principle) - */ - public - Class get(final Class type) { - //noinspection unchecked - final IdentityMap, Class> identityMap = overriddenMethodsREF.get(this); - return identityMap.get(type); - } - - /** - * access a snapshot of the overridden methods (single-writer-principle) - */ - public - Class getReverse(final Class type) { - //noinspection unchecked - final IdentityMap, Class> identityMap = overriddenReverseMethodsREF.get(this); - return identityMap.get(type); - } - - /** - * @throws IllegalArgumentException if the iface/impl have previously been overridden - */ - // synchronized to make sure only one writer at a time - public synchronized - void set(final Class ifaceClass, final Class implClass) { - Class a = this.overriddenMethods.put(ifaceClass, implClass); - Class b = this.overriddenReverseMethods.put(implClass, ifaceClass); - - // this MUST BE UNIQUE otherwise unexpected things can happen. - if (a != null || b != null) { - throw new IllegalArgumentException("Unable to override interface (" + ifaceClass + ") and implementation (" + implClass + ") " + - "because they have already been overridden by something else. It is critical that they are" + - " both unique per JVM"); - } - } -} diff --git a/src/dorkbox/network/rmi/RmiProxyHandler.java b/src/dorkbox/network/rmi/RmiProxyHandler.java index 5515f75c..06d4329b 100644 --- a/src/dorkbox/network/rmi/RmiProxyHandler.java +++ b/src/dorkbox/network/rmi/RmiProxyHandler.java @@ -104,12 +104,9 @@ class RmiProxyHandler implements InvocationHandler { byte responseID = invokeMethodResult.responseID; if (invokeMethodResult.objectID != objectID) { -// System.err.println("FAILED: " + responseID); -// logger.trace("{} FAILED to received data: {} with id ({})", connection, invokeMethodResult.result, invokeMethodResult.responseID); return; } -// logger.trace("{} received data: {} with id ({})", connection, invokeMethodResult.result, invokeMethodResult.responseID); synchronized (this) { if (RmiProxyHandler.this.pendingResponses[responseID]) { RmiProxyHandler.this.responseTable[responseID] = invokeMethodResult;