diff --git a/src/main/java/org/bukkit/craftbukkit/CraftServer.java b/src/main/java/org/bukkit/craftbukkit/CraftServer.java index fbf48c6056..f28b732502 100644 --- a/src/main/java/org/bukkit/craftbukkit/CraftServer.java +++ b/src/main/java/org/bukkit/craftbukkit/CraftServer.java @@ -21,6 +21,7 @@ import java.util.Map; import java.util.Random; import java.util.logging.Level; import java.util.logging.Logger; +import java.lang.InterruptedException; import jline.ConsoleReader; import net.minecraft.server.ChunkCoordinates; import net.minecraft.server.ConvertProgressUpdater; @@ -45,6 +46,8 @@ import org.bukkit.craftbukkit.inventory.CraftFurnaceRecipe; import org.bukkit.craftbukkit.inventory.CraftRecipe; import org.bukkit.craftbukkit.inventory.CraftShapedRecipe; import org.bukkit.craftbukkit.inventory.CraftShapelessRecipe; +import org.bukkit.scheduler.BukkitWorker; +import org.bukkit.scheduler.BukkitTask; import org.bukkit.craftbukkit.scheduler.CraftScheduler; import org.bukkit.util.config.Configuration; @@ -292,6 +295,32 @@ public final class CraftServer implements Server { pluginManager.clearPlugins(); commandMap.clearCommands(); + + int pollCount = 0; + + // Wait for at most 2.5 seconds for plugins to close their threads + while(pollCount < 50 && getScheduler().getActiveWorkers().size() > 0) { + try { + Thread.sleep(50); + } catch (InterruptedException e) { + } + pollCount++; + } + + List overdueWorkers = getScheduler().getActiveWorkers(); + for(BukkitWorker worker : overdueWorkers) { + Plugin plugin = worker.getOwner(); + String author = ""; + if (plugin.getDescription().getAuthors().size() > 0) { + author = plugin.getDescription().getAuthors().get(0); + } + getLogger().log(Level.SEVERE, String.format( + "Nag author: '%s' of '%s' about the following: %s", + author, + plugin.getDescription().getName(), + "This plugin is not properly shutting down its async tasks when it is being reloaded. This may cause conflicts with the newly loaded version of the plugin" + )); + } loadPlugins(); } diff --git a/src/main/java/org/bukkit/craftbukkit/scheduler/CraftScheduler.java b/src/main/java/org/bukkit/craftbukkit/scheduler/CraftScheduler.java index 9caef41619..222a3158f8 100644 --- a/src/main/java/org/bukkit/craftbukkit/scheduler/CraftScheduler.java +++ b/src/main/java/org/bukkit/craftbukkit/scheduler/CraftScheduler.java @@ -3,6 +3,8 @@ package org.bukkit.craftbukkit.scheduler; import java.util.LinkedList; import java.util.TreeMap; import java.util.Iterator; +import java.util.List; +import java.util.ArrayList; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; @@ -12,6 +14,9 @@ import java.util.logging.Level; import java.util.logging.Logger; import org.bukkit.scheduler.BukkitScheduler; +import org.bukkit.scheduler.BukkitWorker; +import org.bukkit.scheduler.BukkitTask; + import org.bukkit.plugin.Plugin; import org.bukkit.craftbukkit.CraftServer; @@ -35,6 +40,7 @@ public class CraftScheduler implements BukkitScheduler, Runnable { // This lock locks the mainThreadQueue and the currentTick value private final Lock mainThreadLock = new ReentrantLock(); + private final Lock syncedTasksLock = new ReentrantLock(); public void run() { @@ -113,28 +119,33 @@ public class CraftScheduler implements BukkitScheduler, Runnable { // If the main thread cannot obtain the lock, it doesn't wait public void mainThreadHeartbeat(long currentTick) { - if (mainThreadLock.tryLock()) { + if (syncedTasksLock.tryLock()) { try { - this.currentTick = currentTick; - while (!mainThreadQueue.isEmpty()) { - syncedTasks.addLast(mainThreadQueue.removeFirst()); - } - } finally { - mainThreadLock.unlock(); - } - while(!syncedTasks.isEmpty()) { - CraftTask task = syncedTasks.removeFirst(); - try { - task.getTask().run(); - } catch (Throwable t) { - // Bad plugin! - logger.log(Level.WARNING, - "Task of '" + task.getOwner().getDescription().getName() - + "' generated an exception", t); - synchronized (schedulerQueue) { - schedulerQueue.remove(task); + if (mainThreadLock.tryLock()) { + try { + this.currentTick = currentTick; + while (!mainThreadQueue.isEmpty()) { + syncedTasks.addLast(mainThreadQueue.removeFirst()); + } + } finally { + mainThreadLock.unlock(); } } + long breakTime = System.currentTimeMillis() + 35; // max time spent in loop = 35ms + while (!syncedTasks.isEmpty() && System.currentTimeMillis() <= breakTime) { + CraftTask task = syncedTasks.removeFirst(); + try { + task.getTask().run(); + } catch (Throwable t) { + // Bad plugin! + logger.log(Level.WARNING, "Task of '" + task.getOwner().getDescription().getName() + "' generated an exception", t); + synchronized (schedulerQueue) { + schedulerQueue.remove(task); + } + } + } + } finally { + syncedTasksLock.unlock(); } } } @@ -159,6 +170,24 @@ public class CraftScheduler implements BukkitScheduler, Runnable { } } + void wipeSyncedTasks() { + syncedTasksLock.lock(); + try { + syncedTasks.clear(); + } finally { + syncedTasksLock.unlock(); + } + } + + void wipeMainThreadQueue() { + mainThreadLock.lock(); + try { + mainThreadQueue.clear(); + } finally { + mainThreadLock.unlock(); + } + } + public int scheduleSyncDelayedTask(Plugin plugin, Runnable task, long delay) { return scheduleSyncRepeatingTask(plugin, task, delay, -1); } @@ -168,10 +197,16 @@ public class CraftScheduler implements BukkitScheduler, Runnable { } public int scheduleSyncRepeatingTask(Plugin plugin, Runnable task, long delay, long period) { - if (plugin == null) throw new IllegalArgumentException("Plugin can not null"); - if (task == null) throw new IllegalArgumentException("Task can not null"); - if (delay < 0) throw new IllegalArgumentException("Delay cannot be less than 0"); - + if (plugin == null) { + throw new IllegalArgumentException("Plugin cannot be null"); + } + if (task == null) { + throw new IllegalArgumentException("Task cannot be null"); + } + if (delay < 0) { + throw new IllegalArgumentException("Delay cannot be less than 0"); + } + CraftTask newTask = new CraftTask(plugin, task, true, getCurrentTick()+delay, period); synchronized (schedulerQueue) { schedulerQueue.put(newTask, true); @@ -189,10 +224,16 @@ public class CraftScheduler implements BukkitScheduler, Runnable { } public int scheduleAsyncRepeatingTask(Plugin plugin, Runnable task, long delay, long period) { - if (plugin == null) throw new IllegalArgumentException("Plugin can not null"); - if (task == null) throw new IllegalArgumentException("Task can not null"); - if (delay < 0) throw new IllegalArgumentException("Delay cannot be less than 0"); - + if (plugin == null) { + throw new IllegalArgumentException("Plugin cannot be null"); + } + if (task == null) { + throw new IllegalArgumentException("Task cannot be null"); + } + if (delay < 0) { + throw new IllegalArgumentException("Delay cannot be less than 0"); + } + CraftTask newTask = new CraftTask(plugin, task, false, getCurrentTick()+delay, period); synchronized (schedulerQueue) { schedulerQueue.put(newTask, false); @@ -203,7 +244,7 @@ public class CraftScheduler implements BukkitScheduler, Runnable { public Future callSyncMethod(Plugin plugin, Callable task) { CraftFuture craftFuture = new CraftFuture(this, task); - synchronized(craftFuture) { + synchronized (craftFuture) { int taskId = scheduleSyncDelayedTask(plugin, craftFuture); craftFuture.setTaskId(taskId); } @@ -224,15 +265,40 @@ public class CraftScheduler implements BukkitScheduler, Runnable { } public void cancelTasks(Plugin plugin) { - synchronized (schedulerQueue) { - Iterator itr = schedulerQueue.keySet().iterator(); - while (itr.hasNext()) { - CraftTask current = itr.next(); - if (current.getOwner().equals(plugin)) { - itr.remove(); + syncedTasksLock.lock(); + try { + synchronized(schedulerQueue) { + mainThreadLock.lock(); + try { + Iterator itr = schedulerQueue.keySet().iterator(); + while (itr.hasNext()) { + CraftTask current = itr.next(); + if (current.getOwner().equals(plugin)) { + itr.remove(); + } + } + itr = mainThreadQueue.iterator(); + while (itr.hasNext()) { + CraftTask current = itr.next(); + if (current.getOwner().equals(plugin)) { + itr.remove(); + } + } + itr = syncedTasks.iterator(); + while (itr.hasNext()) { + CraftTask current = itr.next(); + if (current.getOwner().equals(plugin)) { + itr.remove(); + } + } + } finally { + mainThreadLock.unlock(); } } + } finally { + syncedTasksLock.unlock(); } + craftThreadManager.interruptTasks(plugin); } @@ -240,6 +306,9 @@ public class CraftScheduler implements BukkitScheduler, Runnable { synchronized (schedulerQueue) { schedulerQueue.clear(); } + wipeMainThreadQueue(); + wipeSyncedTasks(); + craftThreadManager.interruptAllTasks(); } @@ -260,4 +329,40 @@ public class CraftScheduler implements BukkitScheduler, Runnable { } } + public List getActiveWorkers() { + synchronized (craftThreadManager.workers) { + List workerList = new ArrayList(craftThreadManager.workers.size()); + Iterator itr = craftThreadManager.workers.iterator(); + while (itr.hasNext()) { + workerList.add((BukkitWorker)itr.next()); + } + return workerList; + } + } + + public List getPendingTasks() { + List taskList = null; + syncedTasksLock.lock(); + try { + synchronized (schedulerQueue) { + mainThreadLock.lock(); + try { + taskList = new ArrayList(mainThreadQueue.size() + syncedTasks.size() + schedulerQueue.size()); + taskList.addAll(mainThreadQueue); + taskList.addAll(syncedTasks); + taskList.addAll(schedulerQueue.keySet()); + } finally { + mainThreadLock.unlock(); + } + } + } finally { + syncedTasksLock.unlock(); + } + List newTaskList = new ArrayList(taskList.size()); + for (CraftTask craftTask : taskList) { + newTaskList.add((BukkitTask)craftTask); + } + return newTaskList; + } + } diff --git a/src/main/java/org/bukkit/craftbukkit/scheduler/CraftTask.java b/src/main/java/org/bukkit/craftbukkit/scheduler/CraftTask.java index 7d5b610e96..d5cc4f4ebe 100644 --- a/src/main/java/org/bukkit/craftbukkit/scheduler/CraftTask.java +++ b/src/main/java/org/bukkit/craftbukkit/scheduler/CraftTask.java @@ -3,8 +3,9 @@ package org.bukkit.craftbukkit.scheduler; import java.lang.Comparable; import org.bukkit.plugin.Plugin; +import org.bukkit.scheduler.BukkitTask; -public class CraftTask implements Comparable { +public class CraftTask implements Comparable, BukkitTask { private final Runnable task; private final boolean syncTask; @@ -44,7 +45,7 @@ public class CraftTask implements Comparable { return task; } - boolean isSync() { + public boolean isSync() { return syncTask; } @@ -56,7 +57,7 @@ public class CraftTask implements Comparable { return period; } - Plugin getOwner() { + public Plugin getOwner() { return owner; } @@ -64,6 +65,10 @@ public class CraftTask implements Comparable { executionTick += period; } + public int getTaskId() { + return getIdNumber(); + } + int getIdNumber() { return idNumber; } @@ -104,6 +109,4 @@ public class CraftTask implements Comparable { public int hashCode() { return getIdNumber(); } - - } diff --git a/src/main/java/org/bukkit/craftbukkit/scheduler/CraftWorker.java b/src/main/java/org/bukkit/craftbukkit/scheduler/CraftWorker.java index 3bf91e4fbf..42f9bb9bea 100644 --- a/src/main/java/org/bukkit/craftbukkit/scheduler/CraftWorker.java +++ b/src/main/java/org/bukkit/craftbukkit/scheduler/CraftWorker.java @@ -1,8 +1,9 @@ package org.bukkit.craftbukkit.scheduler; import org.bukkit.plugin.Plugin; +import org.bukkit.scheduler.BukkitWorker; -public class CraftWorker implements Runnable { +public class CraftWorker implements Runnable, BukkitWorker { private static int hashIdCounter = 1; private static Object hashIdCounterSync = new Object(); @@ -49,6 +50,10 @@ public class CraftWorker implements Runnable { return owner; } + public Thread getThread() { + return t; + } + public void interrupt() { t.interrupt(); } @@ -70,7 +75,6 @@ public class CraftWorker implements Runnable { @Override public boolean equals( Object other ) { - if (other == null) { return false; }