Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve injector listener tracking and fix terminal packets breaking injection #2951

Merged
30 changes: 12 additions & 18 deletions src/main/java/com/comphenix/protocol/CommandProtocol.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.io.FileWriter;
import java.io.IOException;
import java.io.PrintWriter;
import java.nio.file.Path;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.HashSet;
Expand All @@ -35,8 +36,8 @@
import com.comphenix.protocol.error.DetailedErrorReporter;
import com.comphenix.protocol.error.ErrorReporter;
import com.comphenix.protocol.events.PacketListener;
import com.comphenix.protocol.timing.TimedListenerManager;
import com.comphenix.protocol.timing.TimingReportGenerator;
import com.comphenix.protocol.timing.TimingReport;
import com.comphenix.protocol.timing.TimingTrackerManager;
import com.comphenix.protocol.updater.Updater;
import com.comphenix.protocol.updater.Updater.UpdateType;
import com.comphenix.protocol.utility.Closer;
Expand Down Expand Up @@ -143,15 +144,14 @@ public void run() {
}

private void toggleTimings(CommandSender sender, String[] args) {
TimedListenerManager manager = TimedListenerManager.getInstance();
boolean state = !manager.isTiming(); // toggle
boolean isNotTracking = !TimingTrackerManager.isTracking();

// Parse the boolean parameter
if (args.length == 2) {
Boolean parsed = parseBoolean(toQueue(args, 2), "start");

if (parsed != null) {
state = parsed;
isNotTracking = parsed;
} else {
sender.sendMessage(ChatColor.RED + "Specify a state: ON or OFF.");
return;
Expand All @@ -161,31 +161,25 @@ private void toggleTimings(CommandSender sender, String[] args) {
return;
}

// Now change the state
if (state) {
if (manager.startTiming())
if (isNotTracking) {
if (TimingTrackerManager.startTracking())
sender.sendMessage(ChatColor.GOLD + "Started timing packet listeners.");
else
sender.sendMessage(ChatColor.RED + "Packet timing already started.");
} else {
if (manager.stopTiming()) {
saveTimings(manager);
if (TimingTrackerManager.stopTracking()) {
saveTimings(TimingTrackerManager.createReportAndReset());
sender.sendMessage(ChatColor.GOLD + "Stopped and saved result in plugin folder.");
} else {
sender.sendMessage(ChatColor.RED + "Packet timing already stopped.");
}
}
}

private void saveTimings(TimedListenerManager manager) {
private void saveTimings(TimingReport report) {
try {
File destination = new File(plugin.getDataFolder(), "Timings - " + System.currentTimeMillis() + ".txt");
TimingReportGenerator generator = new TimingReportGenerator();

// Print to a text file
generator.saveTo(destination, manager);
manager.clear();

Path path = plugin.getDataFolder().toPath().resolve("timings_" + System.currentTimeMillis() + ".txt");
report.saveTo(path);
} catch (IOException e) {
reporter.reportMinimal(plugin, "saveTimings()", e);
}
Expand Down
61 changes: 27 additions & 34 deletions src/main/java/com/comphenix/protocol/async/AsyncFilterManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@

package com.comphenix.protocol.async;

import java.util.Collection;
import java.util.List;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicInteger;

import org.bukkit.entity.Player;
import org.bukkit.plugin.Plugin;

import com.comphenix.protocol.AsynchronousManager;
import com.comphenix.protocol.PacketStream;
import com.comphenix.protocol.PacketType;
Expand All @@ -31,16 +33,13 @@
import com.comphenix.protocol.events.ListeningWhitelist;
import com.comphenix.protocol.events.PacketEvent;
import com.comphenix.protocol.events.PacketListener;
import com.comphenix.protocol.injector.PrioritizedListener;
import com.comphenix.protocol.injector.SortedPacketListenerList;
import com.comphenix.protocol.injector.collection.InboundPacketListenerSet;
import com.comphenix.protocol.injector.collection.OutboundPacketListenerSet;
import com.comphenix.protocol.scheduler.ProtocolScheduler;
import com.google.common.base.Objects;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;

import org.bukkit.entity.Player;
import org.bukkit.plugin.Plugin;

/**
* Represents a filter manager for asynchronous packets.
* <p>
Expand All @@ -50,8 +49,8 @@
*/
public class AsyncFilterManager implements AsynchronousManager {

private SortedPacketListenerList serverTimeoutListeners;
private SortedPacketListenerList clientTimeoutListeners;
private OutboundPacketListenerSet outboundTimeoutListeners;
private InboundPacketListenerSet inboundTimeoutListeners;
private Set<PacketListener> timeoutListeners;

private PacketProcessingQueue serverProcessingQueue;
Expand Down Expand Up @@ -84,11 +83,11 @@ public class AsyncFilterManager implements AsynchronousManager {
*/
public AsyncFilterManager(ErrorReporter reporter, ProtocolScheduler scheduler) {
// Initialize timeout listeners
this.serverTimeoutListeners = new SortedPacketListenerList();
this.clientTimeoutListeners = new SortedPacketListenerList();
this.outboundTimeoutListeners = new OutboundPacketListenerSet(null, reporter);
this.inboundTimeoutListeners = new InboundPacketListenerSet(null, reporter);
this.timeoutListeners = ConcurrentHashMap.newKeySet();

this.playerSendingHandler = new PlayerSendingHandler(reporter, serverTimeoutListeners, clientTimeoutListeners);
this.playerSendingHandler = new PlayerSendingHandler(outboundTimeoutListeners, inboundTimeoutListeners);
this.serverProcessingQueue = new PacketProcessingQueue(playerSendingHandler);
this.clientProcessingQueue = new PacketProcessingQueue(playerSendingHandler);
this.playerSendingHandler.initializeScheduler();
Expand Down Expand Up @@ -130,9 +129,9 @@ public void registerTimeoutHandler(PacketListener listener) {
ListeningWhitelist receiving = listener.getReceivingWhitelist();

if (!ListeningWhitelist.isEmpty(sending))
serverTimeoutListeners.addListener(listener, sending);
outboundTimeoutListeners.addListener(listener);
if (!ListeningWhitelist.isEmpty(receiving))
serverTimeoutListeners.addListener(listener, receiving);
inboundTimeoutListeners.addListener(listener);
}

@Override
Expand All @@ -145,9 +144,8 @@ public Set<PacketListener> getAsyncHandlers() {
ImmutableSet.Builder<PacketListener> builder = ImmutableSet.builder();

// Add every asynchronous packet listener
for (PrioritizedListener<AsyncListenerHandler> handler :
Iterables.concat(serverProcessingQueue.values(), clientProcessingQueue.values())) {
builder.add(handler.getListener().getAsyncListener());
for (AsyncListenerHandler handler : Iterables.concat(serverProcessingQueue.values(), clientProcessingQueue.values())) {
builder.add(handler.getAsyncListener());
}
return builder.build();
}
Expand Down Expand Up @@ -203,13 +201,9 @@ public void unregisterTimeoutHandler(PacketListener listener) {
if (listener == null)
throw new IllegalArgumentException("listener cannot be NULL.");

ListeningWhitelist sending = listener.getSendingWhitelist();
ListeningWhitelist receiving = listener.getReceivingWhitelist();

// Do it in the opposite order
if (serverTimeoutListeners.removeListener(listener, sending).size() > 0 ||
clientTimeoutListeners.removeListener(listener, receiving).size() > 0) {
timeoutListeners.remove(listener);
if (timeoutListeners.remove(listener)) {
outboundTimeoutListeners.removeListener(listener);
inboundTimeoutListeners.removeListener(listener);
}
}

Expand All @@ -233,9 +227,9 @@ private AsyncListenerHandler findHandler(PacketProcessingQueue queue, ListeningW
return null;

for (PacketType type : search.getTypes()) {
for (PrioritizedListener<AsyncListenerHandler> element : queue.getListener(type)) {
if (element.getListener().getAsyncListener() == target) {
return element.getListener();
for (AsyncListenerHandler element : queue.get(type)) {
if (element.getAsyncListener() == target) {
return element;
}
}
}
Expand Down Expand Up @@ -292,10 +286,10 @@ public void unregisterAsyncHandlers(Plugin plugin) {
private void unregisterAsyncHandlers(PacketProcessingQueue processingQueue, Plugin plugin) {

// Iterate through every packet listener
for (PrioritizedListener<AsyncListenerHandler> listener : processingQueue.values()) {
for (AsyncListenerHandler listener : processingQueue.values()) {
// Remove the listener
if (Objects.equal(listener.getListener().getPlugin(), plugin)) {
unregisterAsyncHandler(listener.getListener());
if (Objects.equal(listener.getPlugin(), plugin)) {
unregisterAsyncHandler(listener);
}
}
}
Expand Down Expand Up @@ -346,8 +340,7 @@ public ProtocolScheduler getScheduler() {

@Override
public boolean hasAsynchronousListeners(PacketEvent packet) {
Collection<?> list = getProcessingQueue(packet).getListener(packet.getPacketType());
return list != null && list.size() > 0;
return getProcessingQueue(packet).contains(packet.getPacketType());
}

/**
Expand Down Expand Up @@ -388,8 +381,8 @@ public void cleanupAll() {
playerSendingHandler.cleanupAll();
timeoutListeners.clear();

serverTimeoutListeners = null;
clientTimeoutListeners = null;
outboundTimeoutListeners = null;
inboundTimeoutListeners = null;
}

@Override
Expand Down Expand Up @@ -417,7 +410,7 @@ private void signalPacketTransmission(PacketEvent packet, boolean onMainThread)
// Now, get the next non-cancelled listener
if (!marker.hasExpired()) {
for (; marker.getListenerTraversal().hasNext(); ) {
AsyncListenerHandler handler = marker.getListenerTraversal().next().getListener();
AsyncListenerHandler handler = marker.getListenerTraversal().next();

if (!handler.isCancelled()) {
marker.incrementProcessingDelay();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,8 @@
import com.comphenix.protocol.events.PacketEvent;
import com.comphenix.protocol.events.PacketListener;
import com.comphenix.protocol.scheduler.Task;
import com.comphenix.protocol.timing.TimedListenerManager;
import com.comphenix.protocol.timing.TimedListenerManager.ListenerType;
import com.comphenix.protocol.timing.TimedTracker;
import com.comphenix.protocol.timing.TimingListenerType;
import com.comphenix.protocol.timing.TimingTrackerManager;
import com.google.common.base.Function;
import com.google.common.base.Joiner;

Expand Down Expand Up @@ -100,9 +99,6 @@ public class AsyncListenerHandler {
// Warn plugins that the async listener handler must be started
private Task warningTask;

// Timing manager
private TimedListenerManager timedManager = TimedListenerManager.getInstance();

/**
* Construct a manager for an asynchronous packet handler.
* @param mainThread - the main game thread.
Expand Down Expand Up @@ -600,27 +596,13 @@ private void processPacket(int workerID, PacketEvent packet, String methodName)
marker.setListenerHandler(this);
marker.setWorkerID(workerID);

// We're not THAT worried about performance here
if (timedManager.isTiming()) {
// Retrieve the tracker to use
TimedTracker tracker = timedManager.getTracker(listener,
packet.isServerPacket() ? ListenerType.ASYNC_SERVER_SIDE : ListenerType.ASYNC_CLIENT_SIDE);
long token = tracker.beginTracking();

if (packet.isServerPacket())
listener.onPacketSending(packet);
else
listener.onPacketReceiving(packet);

// And we're done
tracker.endTracking(token, packet.getPacketType());

} else {
if (packet.isServerPacket())
listener.onPacketSending(packet);
else
listener.onPacketReceiving(packet);
}
TimingTrackerManager.get(listener, packet.isServerPacket() ? TimingListenerType.ASYNC_OUTBOUND : TimingListenerType.ASYNC_INBOUND)
.track(packet.getPacketType(), () -> {
if (packet.isServerPacket())
listener.onPacketSending(packet);
else
listener.onPacketReceiving(packet);
});
}

} catch (OutOfMemoryError e) {
Expand Down
7 changes: 3 additions & 4 deletions src/main/java/com/comphenix/protocol/async/AsyncMarker.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import com.comphenix.protocol.ProtocolLogger;
import com.comphenix.protocol.events.NetworkMarker;
import com.comphenix.protocol.events.PacketEvent;
import com.comphenix.protocol.injector.PrioritizedListener;
import com.comphenix.protocol.reflect.FieldAccessException;
import com.comphenix.protocol.reflect.FuzzyReflection;
import com.comphenix.protocol.utility.MinecraftReflection;
Expand Down Expand Up @@ -70,7 +69,7 @@ public class AsyncMarker implements Serializable, Comparable<AsyncMarker> {
/**
* Current list of async packet listeners.
*/
private transient Iterator<PrioritizedListener<AsyncListenerHandler>> listenerTraversal;
private transient Iterator<AsyncListenerHandler> listenerTraversal;

// Timeout handling
private long initialTime;
Expand Down Expand Up @@ -366,15 +365,15 @@ void setWorkerID(int workerID) {
* Retrieve iterator for the next listener in line.
* @return Next async packet listener iterator.
*/
Iterator<PrioritizedListener<AsyncListenerHandler>> getListenerTraversal() {
Iterator<AsyncListenerHandler> getListenerTraversal() {
return listenerTraversal;
}

/**
* Set the iterator for the next listener.
* @param listenerTraversal - the new async packet listener iterator.
*/
void setListenerTraversal(Iterator<PrioritizedListener<AsyncListenerHandler>> listenerTraversal) {
void setListenerTraversal(Iterator<AsyncListenerHandler> listenerTraversal) {
this.listenerTraversal = listenerTraversal;
}

Expand Down
Loading
Loading