From 092800af2690a46fb36783f75b7575a3ece2745c Mon Sep 17 00:00:00 2001 From: Wesley Wolfe Date: Wed, 14 Nov 2012 16:47:21 -0600 Subject: Fixed some async tasks running synchronously. Fixes BUKKIT-2934 Additionally refactored cancel method to be more object-oriented. --- .../craftbukkit/scheduler/CraftAsyncTask.java | 8 ++++ .../bukkit/craftbukkit/scheduler/CraftFuture.java | 9 ++++ .../craftbukkit/scheduler/CraftScheduler.java | 49 +++++----------------- .../bukkit/craftbukkit/scheduler/CraftTask.java | 10 +++++ 4 files changed, 37 insertions(+), 39 deletions(-) (limited to 'src/main/java') diff --git a/src/main/java/org/bukkit/craftbukkit/scheduler/CraftAsyncTask.java b/src/main/java/org/bukkit/craftbukkit/scheduler/CraftAsyncTask.java index 6901e4ee..75a1a63a 100644 --- a/src/main/java/org/bukkit/craftbukkit/scheduler/CraftAsyncTask.java +++ b/src/main/java/org/bukkit/craftbukkit/scheduler/CraftAsyncTask.java @@ -95,4 +95,12 @@ class CraftAsyncTask extends CraftTask { LinkedList getWorkers() { return workers; } + + boolean cancel0() { + synchronized (workers) { + // Synchronizing here prevents race condition for a completing task + setPeriod(-2l); + } + return true; + } } diff --git a/src/main/java/org/bukkit/craftbukkit/scheduler/CraftFuture.java b/src/main/java/org/bukkit/craftbukkit/scheduler/CraftFuture.java index fee49c92..1baec564 100644 --- a/src/main/java/org/bukkit/craftbukkit/scheduler/CraftFuture.java +++ b/src/main/java/org/bukkit/craftbukkit/scheduler/CraftFuture.java @@ -96,4 +96,13 @@ class CraftFuture extends CraftTask implements Future { } } } + + synchronized boolean cancel0() { + if (getPeriod() != -1l) { + return false; + } + setPeriod(-2l); + notifyAll(); + return true; + } } diff --git a/src/main/java/org/bukkit/craftbukkit/scheduler/CraftScheduler.java b/src/main/java/org/bukkit/craftbukkit/scheduler/CraftScheduler.java index 15b15dbf..c10008fc 100644 --- a/src/main/java/org/bukkit/craftbukkit/scheduler/CraftScheduler.java +++ b/src/main/java/org/bukkit/craftbukkit/scheduler/CraftScheduler.java @@ -108,7 +108,7 @@ public class CraftScheduler implements BukkitScheduler { } public BukkitTask runTaskLaterAsynchronously(Plugin plugin, Runnable runnable, long delay) { - return runTaskTimer(plugin, runnable, delay, -1l); + return runTaskTimerAsynchronously(plugin, runnable, delay, -1l); } public int scheduleSyncRepeatingTask(final Plugin plugin, final Runnable runnable, long delay, long period) { @@ -158,7 +158,7 @@ public class CraftScheduler implements BukkitScheduler { } CraftTask task = runners.get(taskId); if (task != null) { - cancelTask(task); + task.cancel0(); } task = new CraftTask( new Runnable() { @@ -172,7 +172,7 @@ public class CraftScheduler implements BukkitScheduler { while (tasks.hasNext()) { final CraftTask task = tasks.next(); if (task.getTaskId() == taskId) { - cancelTask(task); + task.cancel0(); tasks.remove(); if (task.isSync()) { runners.remove(taskId); @@ -188,7 +188,7 @@ public class CraftScheduler implements BukkitScheduler { return; } if (taskPending.getTaskId() == taskId) { - cancelTask(taskPending); + taskPending.cancel0(); } } } @@ -206,7 +206,7 @@ public class CraftScheduler implements BukkitScheduler { while (tasks.hasNext()) { final CraftTask task = tasks.next(); if (task.getOwner().equals(plugin)) { - cancelTask(task); + task.cancel0(); tasks.remove(); if (task.isSync()) { runners.remove(task.getTaskId()); @@ -222,12 +222,12 @@ public class CraftScheduler implements BukkitScheduler { return; } if (taskPending.getTaskId() != -1 && taskPending.getOwner().equals(plugin)) { - cancelTask(taskPending); + taskPending.cancel0(); } } for (CraftTask runner : runners.values()) { if (runner.getOwner().equals(plugin)) { - cancelTask(runner); + runner.cancel0(); } } } @@ -239,7 +239,7 @@ public class CraftScheduler implements BukkitScheduler { Iterator it = CraftScheduler.this.runners.values().iterator(); while (it.hasNext()) { CraftTask task = it.next(); - cancelTask(task); + task.cancel0(); if (task.isSync()) { it.remove(); } @@ -253,10 +253,10 @@ public class CraftScheduler implements BukkitScheduler { if (taskPending == task) { break; } - cancelTask(taskPending); + taskPending.cancel0(); } for (CraftTask runner : runners.values()) { - cancelTask(runner); + runner.cancel0(); } } @@ -421,35 +421,6 @@ public class CraftScheduler implements BukkitScheduler { return !pending.isEmpty() && pending.peek().getNextRun() <= currentTick; } - /** - * This method is important to make sure the code is consistent everywhere. - * Synchronizing is needed for future and async to prevent race conditions, - * main thread or otherwise. - * @return True if cancelled - */ - private boolean cancelTask(final CraftTask task) { - if (task.isSync()) { - if (task instanceof CraftFuture) { - synchronized (task) { - if (task.getPeriod() != -1l) { - return false; - } - // This needs to be set INSIDE of the synchronized block - task.setPeriod(-2l); - task.notifyAll(); - } - } else { - task.setPeriod(-2l); - } - } else { - synchronized (((CraftAsyncTask) task).getWorkers()) { - // Synchronizing here prevents race condition for a completing task - task.setPeriod(-2l); - } - } - return true; - } - @Override public String toString() { int debugTick = currentTick; diff --git a/src/main/java/org/bukkit/craftbukkit/scheduler/CraftTask.java b/src/main/java/org/bukkit/craftbukkit/scheduler/CraftTask.java index 4dd1cb29..55db3ff8 100644 --- a/src/main/java/org/bukkit/craftbukkit/scheduler/CraftTask.java +++ b/src/main/java/org/bukkit/craftbukkit/scheduler/CraftTask.java @@ -84,4 +84,14 @@ class CraftTask implements BukkitTask, Runnable { public void cancel() { Bukkit.getScheduler().cancelTask(id); } + + /** + * This method properly sets the status to cancelled, synchronizing when required. + * + * @return false if it is a craft future task that has already begun execution, true otherwise + */ + boolean cancel0() { + setPeriod(-2l); + return true; + } } -- cgit v1.2.3