Archiviert
13
0

Fixed a serious misuse causing the creation of too many new classes.

The different injectors using CGLib was using a custom CallbackFilter
to optimize the resulting generated class. Unfortunately, the custom
filter didn't properly implement equals() and hashCode(), and so every
time a player logged, a new injector class had to be generated. 

This was fixed by making the injectors share a single CallbackFilter.

In addition, I've begun adding cleanup code that will reset all static
fields once the plugin has unloaded.
Dieser Commit ist enthalten in:
Kristian S. Stangeland 2012-10-29 01:42:22 +01:00
Ursprung eaf0b73c00
Commit bdc41221db
7 geänderte Dateien mit 61 neuen und 32 gelöschten Zeilen

Datei anzeigen

@ -4,7 +4,7 @@
<groupId>com.comphenix.protocol</groupId> <groupId>com.comphenix.protocol</groupId>
<artifactId>ProtocolLib</artifactId> <artifactId>ProtocolLib</artifactId>
<name>ProtocolLib</name> <name>ProtocolLib</name>
<version>1.4.3-SNAPSHOT</version> <version>1.4.4-SNAPSHOT</version>
<description>Provides read/write access to the Minecraft protocol.</description> <description>Provides read/write access to the Minecraft protocol.</description>
<url>http://dev.bukkit.org/server-mods/protocollib/</url> <url>http://dev.bukkit.org/server-mods/protocollib/</url>
<developers> <developers>

Datei anzeigen

@ -190,6 +190,10 @@ public class ProtocolLibrary extends JavaPlugin {
protocolManager.close(); protocolManager.close();
protocolManager = null; protocolManager = null;
statistisc = null; statistisc = null;
// Leaky ClassLoader begone!
CleanupStaticMembers cleanup = new CleanupStaticMembers(getClassLoader(), logger);
cleanup.resetAll();
} }
/** /**

Datei anzeigen

@ -54,7 +54,7 @@ public class AsyncListenerHandler {
private static final AtomicInteger nextID = new AtomicInteger(); private static final AtomicInteger nextID = new AtomicInteger();
// Default queue capacity // Default queue capacity
private static int DEFAULT_CAPACITY = 1024; private static final int DEFAULT_CAPACITY = 1024;
// Cancel the async handler // Cancel the async handler
private volatile boolean cancelled; private volatile boolean cancelled;

Datei anzeigen

@ -41,7 +41,7 @@ public class PacketConstructor {
* <p> * <p>
* Remember to call withPacket(). * Remember to call withPacket().
*/ */
public static final PacketConstructor DEFAULT = new PacketConstructor(null); public static PacketConstructor DEFAULT = new PacketConstructor(null);
// The constructor method that's actually responsible for creating the packet // The constructor method that's actually responsible for creating the packet
private Constructor<?> constructorMethod; private Constructor<?> constructorMethod;

Datei anzeigen

@ -50,6 +50,9 @@ class NetworkObjectInjector extends PlayerInjector {
// Used to construct proxy objects // Used to construct proxy objects
private ClassLoader classLoader; private ClassLoader classLoader;
// Shared callback filter - avoid creating a new class every time
private static CallbackFilter callbackFilter;
public NetworkObjectInjector(ClassLoader classLoader, Logger logger, Player player, public NetworkObjectInjector(ClassLoader classLoader, Logger logger, Player player,
ListenerInvoker invoker, IntegerSet sendingFilters) throws IllegalAccessException { ListenerInvoker invoker, IntegerSet sendingFilters) throws IllegalAccessException {
@ -131,20 +134,25 @@ class NetworkObjectInjector extends PlayerInjector {
} }
}; };
// Share callback filter - that way, we avoid generating a new class every time.
if (callbackFilter == null) {
callbackFilter = new CallbackFilter() {
@Override
public int accept(Method method) {
if (method.equals(queueMethod))
return 0;
else
return 1;
}
};
}
// Create our proxy object // Create our proxy object
Enhancer ex = new Enhancer(); Enhancer ex = new Enhancer();
ex.setClassLoader(classLoader); ex.setClassLoader(classLoader);
ex.setSuperclass(networkInterface); ex.setSuperclass(networkInterface);
ex.setCallbacks(new Callback[] { queueFilter, dispatch }); ex.setCallbacks(new Callback[] { queueFilter, dispatch });
ex.setCallbackFilter(new CallbackFilter() { ex.setCallbackFilter(callbackFilter);
@Override
public int accept(Method method) {
if (method.equals(queueMethod))
return 0;
else
return 1;
}
});
// Inject it, if we can. // Inject it, if we can.
networkManagerRef.setValue(ex.create()); networkManagerRef.setValue(ex.create());

Datei anzeigen

@ -52,6 +52,8 @@ import com.comphenix.protocol.reflect.instances.ExistingGenerator;
*/ */
public class NetworkServerInjector extends PlayerInjector { public class NetworkServerInjector extends PlayerInjector {
private volatile static CallbackFilter callbackFilter;
private static Field disconnectField; private static Field disconnectField;
private static Method sendPacketMethod; private static Method sendPacketMethod;
private InjectedServerConnection serverInjection; private InjectedServerConnection serverInjection;
@ -170,18 +172,24 @@ public class NetworkServerInjector extends PlayerInjector {
}; };
Callback noOpCallback = NoOp.INSTANCE; Callback noOpCallback = NoOp.INSTANCE;
// Share callback filter - that way, we avoid generating a new class for
// every logged in player.
if (callbackFilter == null) {
callbackFilter = new CallbackFilter() {
@Override
public int accept(Method method) {
if (method.equals(sendPacketMethod))
return 0;
else
return 1;
}
};
}
ex.setClassLoader(classLoader); ex.setClassLoader(classLoader);
ex.setSuperclass(serverClass); ex.setSuperclass(serverClass);
ex.setCallbacks(new Callback[] { sendPacketCallback, noOpCallback }); ex.setCallbacks(new Callback[] { sendPacketCallback, noOpCallback });
ex.setCallbackFilter(new CallbackFilter() { ex.setCallbackFilter(callbackFilter);
@Override
public int accept(Method method) {
if (method.equals(sendPacketMethod))
return 0;
else
return 1;
}
});
// Find the Minecraft NetServerHandler superclass // Find the Minecraft NetServerHandler superclass
Class<?> minecraftSuperClass = getFirstMinecraftSuperClass(serverHandler.getClass()); Class<?> minecraftSuperClass = getFirstMinecraftSuperClass(serverHandler.getClass());
@ -208,6 +216,7 @@ public class NetworkServerInjector extends PlayerInjector {
if (proxyObject != null) { if (proxyObject != null) {
// This will be done by InjectedServerConnection instead // This will be done by InjectedServerConnection instead
//copyTo(serverHandler, proxyObject); //copyTo(serverHandler, proxyObject);
serverInjection.replaceServerHandler(serverHandler, proxyObject); serverInjection.replaceServerHandler(serverHandler, proxyObject);
serverHandlerRef.setValue(proxyObject); serverHandlerRef.setValue(proxyObject);
return true; return true;

Datei anzeigen

@ -42,6 +42,9 @@ class TemporaryPlayerFactory {
// Helpful constructors // Helpful constructors
private final PacketConstructor chatPacket; private final PacketConstructor chatPacket;
// Prevent too many class creations
private static CallbackFilter callbackFilter;
public TemporaryPlayerFactory() { public TemporaryPlayerFactory() {
chatPacket = PacketConstructor.DEFAULT.withPacket(3, new Object[] { "DEMO" }); chatPacket = PacketConstructor.DEFAULT.withPacket(3, new Object[] { "DEMO" });
} }
@ -120,22 +123,27 @@ class TemporaryPlayerFactory {
} }
}; };
// Shared callback filter
if (callbackFilter == null) {
callbackFilter = new CallbackFilter() {
@Override
public int accept(Method method) {
// Do not override the object method or the superclass methods
if (method.getDeclaringClass().equals(Object.class) ||
method.getDeclaringClass().equals(InjectContainer.class))
return 0;
else
return 1;
}
};
}
// CGLib is amazing // CGLib is amazing
Enhancer ex = new Enhancer(); Enhancer ex = new Enhancer();
ex.setSuperclass(InjectContainer.class); ex.setSuperclass(InjectContainer.class);
ex.setInterfaces(new Class[] { Player.class }); ex.setInterfaces(new Class[] { Player.class });
ex.setCallbacks(new Callback[] { NoOp.INSTANCE, implementation }); ex.setCallbacks(new Callback[] { NoOp.INSTANCE, implementation });
ex.setCallbackFilter(new CallbackFilter() { ex.setCallbackFilter(callbackFilter);
@Override
public int accept(Method method) {
// Do not override the object method or the superclass methods
if (method.getDeclaringClass().equals(Object.class) ||
method.getDeclaringClass().equals(InjectContainer.class))
return 0;
else
return 1;
}
});
return (Player) ex.create(); return (Player) ex.create();
} }