summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorScore_Under <seejay.11@gmail.com>2012-07-05 23:51:12 +0100
committerWesley Wolfe <weswolf@aol.com>2013-08-07 02:04:31 -0500
commit84c6c70b9003433714552ca733a6b38ad5c84208 (patch)
tree85d0519884dcafd418d5ae23b67f3de6b19b3fca
parente5a14e2dcc4bd5eafc5de1124898e2393df2d945 (diff)
downloadbukkit-84c6c70b9003433714552ca733a6b38ad5c84208.tar
bukkit-84c6c70b9003433714552ca733a6b38ad5c84208.tar.gz
bukkit-84c6c70b9003433714552ca733a6b38ad5c84208.tar.lz
bukkit-84c6c70b9003433714552ca733a6b38ad5c84208.tar.xz
bukkit-84c6c70b9003433714552ca733a6b38ad5c84208.zip
[BREAKING] Use event class instead of event for timings. Fixes BUKKIT-4664
TimedRegisteredListener uses a reference to the first event fired. This causes a memory leak in the server for any references that event has. This changes TimedRegisteredListener to only store a reference to the class of the event. This change is intentionally a breaking change, as it is an obscure part of the API. A non-breaking change would require the leak to be maintained or an immediate update for any plugins using the method, as it would be an indirect break. A unit test is also included to check behavior of shared superclass functionality.
-rw-r--r--src/main/java/org/bukkit/command/defaults/TimingsCommand.java6
-rw-r--r--src/main/java/org/bukkit/plugin/TimedRegisteredListener.java38
-rw-r--r--src/test/java/org/bukkit/plugin/TimedRegisteredListenerTest.java56
3 files changed, 86 insertions, 14 deletions
diff --git a/src/main/java/org/bukkit/command/defaults/TimingsCommand.java b/src/main/java/org/bukkit/command/defaults/TimingsCommand.java
index 29ebbe05..05cfcb01 100644
--- a/src/main/java/org/bukkit/command/defaults/TimingsCommand.java
+++ b/src/main/java/org/bukkit/command/defaults/TimingsCommand.java
@@ -84,9 +84,9 @@ public class TimingsCommand extends BukkitCommand {
if (count == 0) continue;
long avg = time / count;
totalTime += time;
- Event event = trl.getEvent();
- if (count > 0 && event != null) {
- fileTimings.println(" " + event.getClass().getSimpleName() + (trl.hasMultiple() ? " (and others)" : "") + " Time: " + time + " Count: " + count + " Avg: " + avg);
+ Class<? extends Event> eventClass = trl.getEventClass();
+ if (count > 0 && eventClass != null) {
+ fileTimings.println(" " + eventClass.getSimpleName() + (trl.hasMultiple() ? " (and sub-classes)" : "") + " Time: " + time + " Count: " + count + " Avg: " + avg);
}
}
}
diff --git a/src/main/java/org/bukkit/plugin/TimedRegisteredListener.java b/src/main/java/org/bukkit/plugin/TimedRegisteredListener.java
index ed25e179..d86805b9 100644
--- a/src/main/java/org/bukkit/plugin/TimedRegisteredListener.java
+++ b/src/main/java/org/bukkit/plugin/TimedRegisteredListener.java
@@ -11,7 +11,7 @@ import org.bukkit.event.Listener;
public class TimedRegisteredListener extends RegisteredListener {
private int count;
private long totalTime;
- private Event event;
+ private Class<? extends Event> eventClass;
private boolean multiple = false;
public TimedRegisteredListener(final Listener pluginListener, final EventExecutor eventExecutor, final EventPriority eventPriority, final Plugin registeredPlugin, final boolean listenCancelled) {
@@ -25,17 +25,25 @@ public class TimedRegisteredListener extends RegisteredListener {
return;
}
count++;
- if (this.event == null) {
- this.event = event;
- }
- else if (!this.event.getClass().equals(event.getClass())) {
+ Class<? extends Event> newEventClass = event.getClass();
+ if (this.eventClass == null) {
+ this.eventClass = newEventClass;
+ } else if (!this.eventClass.equals(newEventClass)) {
multiple = true;
+ this.eventClass = getCommonSuperclass(newEventClass, this.eventClass).asSubclass(Event.class);
}
long start = System.nanoTime();
super.callEvent(event);
totalTime += System.nanoTime() - start;
}
+ private static Class<?> getCommonSuperclass(Class<?> class1, Class<?> class2) {
+ while (!class1.isAssignableFrom(class2)) {
+ class1 = class1.getSuperclass();
+ }
+ return class1;
+ }
+
/**
* Resets the call count and total time for this listener
*/
@@ -63,18 +71,26 @@ public class TimedRegisteredListener extends RegisteredListener {
}
/**
- * Gets the first event this listener handled
+ * Gets the class of the events this listener handled. If it handled
+ * multiple classes of event, the closest shared superclass will be
+ * returned, such that for any event this listener has handled,
+ * <code>this.getEventClass().isAssignableFrom(event.getClass())</code>
+ * and no class
+ * <code>this.getEventClass().isAssignableFrom(clazz)
+ * && this.getEventClass() != clazz &&
+ * event.getClass().isAssignableFrom(clazz)</code> for all handled events.
*
- * @return An event handled by this RegisteredListener
+ * @return the event class handled by this RegisteredListener
*/
- public Event getEvent() {
- return event;
+ public Class<? extends Event> getEventClass() {
+ return eventClass;
}
/**
- * Gets whether this listener has handled multiple events
+ * Gets whether this listener has handled multiple events, such that for
+ * some two events, <code>eventA.getClass() != eventB.getClass()</code>.
*
- * @return True if this listener has handled multiple events
+ * @return true if this listener has handled multiple events
*/
public boolean hasMultiple() {
return multiple;
diff --git a/src/test/java/org/bukkit/plugin/TimedRegisteredListenerTest.java b/src/test/java/org/bukkit/plugin/TimedRegisteredListenerTest.java
new file mode 100644
index 00000000..b206b1f3
--- /dev/null
+++ b/src/test/java/org/bukkit/plugin/TimedRegisteredListenerTest.java
@@ -0,0 +1,56 @@
+package org.bukkit.plugin;
+
+import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.*;
+
+import org.bukkit.event.Event;
+import org.bukkit.event.EventException;
+import org.bukkit.event.EventPriority;
+import org.bukkit.event.Listener;
+import org.bukkit.event.block.BlockBreakEvent;
+import org.bukkit.event.player.PlayerEvent;
+import org.bukkit.event.player.PlayerInteractEvent;
+import org.bukkit.event.player.PlayerMoveEvent;
+import org.junit.Test;
+
+public class TimedRegisteredListenerTest {
+
+ @Test
+ public void testEventClass() throws EventException {
+ Listener listener = new Listener() {};
+ EventExecutor executor = new EventExecutor() {
+ public void execute(Listener listener, Event event) {}
+ };
+ TestPlugin plugin = new TestPlugin("Test");
+
+ PlayerInteractEvent interactEvent = new PlayerInteractEvent(null, null, null, null, null);
+ PlayerMoveEvent moveEvent = new PlayerMoveEvent(null, null, null);
+ BlockBreakEvent breakEvent = new BlockBreakEvent(null, null);
+
+ TimedRegisteredListener trl = new TimedRegisteredListener(listener, executor, EventPriority.NORMAL, plugin, false);
+
+ // Ensure that the correct event type is reported for a single event
+ trl.callEvent(interactEvent);
+ assertThat(trl.getEventClass(), is((Object) PlayerInteractEvent.class));
+ // Ensure that no superclass is used in lieu of the actual event, after two identical event types
+ trl.callEvent(interactEvent);
+ assertThat(trl.getEventClass(), is((Object) PlayerInteractEvent.class));
+ // Ensure that the closest superclass of the two events is chosen
+ trl.callEvent(moveEvent);
+ assertThat(trl.getEventClass(), is((Object) PlayerEvent.class));
+ // As above, so below
+ trl.callEvent(breakEvent);
+ assertThat(trl.getEventClass(), is((Object) Event.class));
+ // In the name of being thorough, check that it never travels down the hierarchy again.
+ trl.callEvent(breakEvent);
+ assertThat(trl.getEventClass(), is((Object) Event.class));
+
+ trl = new TimedRegisteredListener(listener, executor, EventPriority.NORMAL, plugin, false);
+
+ trl.callEvent(breakEvent);
+ assertThat(trl.getEventClass(), is((Object) BlockBreakEvent.class));
+ // Test moving up the class hierarchy by more than one class at a time
+ trl.callEvent(moveEvent);
+ assertThat(trl.getEventClass(), is((Object) Event.class));
+ }
+}