From 71d4016a4d6e70f33e7069de1c64425efa752264 Mon Sep 17 00:00:00 2001 From: CraftBukkit/Spigot Date: Sat, 17 Jul 2021 11:39:56 +1000 Subject: [PATCH] #891: Fix scheduler task ID overflow and duplication issues By: Phoenix616 --- .../craftbukkit/scheduler/CraftScheduler.java | 26 ++++++++++++++++--- .../craftbukkit/scheduler/CraftTask.java | 5 ++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/scheduler/CraftScheduler.java b/paper-server/src/main/java/org/bukkit/craftbukkit/scheduler/CraftScheduler.java index ce2e3d2824..cd43b45516 100644 --- a/paper-server/src/main/java/org/bukkit/craftbukkit/scheduler/CraftScheduler.java +++ b/paper-server/src/main/java/org/bukkit/craftbukkit/scheduler/CraftScheduler.java @@ -14,6 +14,7 @@ import java.util.concurrent.Future; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; +import java.util.function.IntUnaryOperator; import java.util.logging.Level; import org.apache.commons.lang.Validate; import org.bukkit.plugin.IllegalPluginAccessException; @@ -43,10 +44,24 @@ import org.bukkit.scheduler.BukkitWorker; */ public class CraftScheduler implements BukkitScheduler { + /** + * The start ID for the counter. + */ + private static final int START_ID = 1; + /** + * Increment the {@link #ids} field and reset it to the {@link #START_ID} if it reaches {@link Integer#MAX_VALUE} + */ + private static final IntUnaryOperator INCREMENT_IDS = previous -> { + // We reached the end, go back to the start! + if (previous == Integer.MAX_VALUE) { + return START_ID; + } + return previous + 1; + }; /** * Counter for IDs. Order doesn't matter, only uniqueness. */ - private final AtomicInteger ids = new AtomicInteger(1); + private final AtomicInteger ids = new AtomicInteger(START_ID); /** * Current head of linked-list. This reference is always stale, {@link CraftTask#next} is the live reference. */ @@ -65,7 +80,7 @@ public class CraftScheduler implements BukkitScheduler { int value = Long.compare(o1.getNextRun(), o2.getNextRun()); // If the tasks should run on the same tick they should be run FIFO - return value != 0 ? value : Integer.compare(o1.getTaskId(), o2.getTaskId()); + return value != 0 ? value : Long.compare(o1.getCreatedAt(), o2.getCreatedAt()); } }); /** @@ -453,7 +468,12 @@ public class CraftScheduler implements BukkitScheduler { } private int nextId() { - return ids.incrementAndGet(); + Validate.isTrue(runners.size() < Integer.MAX_VALUE, "There are already " + Integer.MAX_VALUE + " tasks scheduled! Cannot schedule more."); + int id; + do { + id = ids.updateAndGet(INCREMENT_IDS); + } while (runners.containsKey(id)); // Avoid generating duplicate IDs + return id; } private void parsePending() { diff --git a/paper-server/src/main/java/org/bukkit/craftbukkit/scheduler/CraftTask.java b/paper-server/src/main/java/org/bukkit/craftbukkit/scheduler/CraftTask.java index b67f2deb80..c885bc7443 100644 --- a/paper-server/src/main/java/org/bukkit/craftbukkit/scheduler/CraftTask.java +++ b/paper-server/src/main/java/org/bukkit/craftbukkit/scheduler/CraftTask.java @@ -27,6 +27,7 @@ class CraftTask implements BukkitTask, Runnable { private final Consumer cTask; private final Plugin plugin; private final int id; + private final long createdAt = System.nanoTime(); CraftTask() { this(null, null, CraftTask.NO_REPEATING, CraftTask.NO_REPEATING); @@ -79,6 +80,10 @@ class CraftTask implements BukkitTask, Runnable { } } + long getCreatedAt() { + return createdAt; + } + long getPeriod() { return period; }