-
Notifications
You must be signed in to change notification settings - Fork 0
Handle Folia scheduler UnsupportedOperationException in Bukkit task flow #2
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,21 +2,32 @@ | |||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| import com.fastasyncworldedit.core.util.TaskManager; | ||||||||||||||||||||||||||||||
| import org.bukkit.Bukkit; | ||||||||||||||||||||||||||||||
| import org.bukkit.Server; | ||||||||||||||||||||||||||||||
| import org.bukkit.plugin.Plugin; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| import javax.annotation.Nonnull; | ||||||||||||||||||||||||||||||
| import java.lang.reflect.Method; | ||||||||||||||||||||||||||||||
| import java.util.Map; | ||||||||||||||||||||||||||||||
| import java.util.concurrent.ConcurrentHashMap; | ||||||||||||||||||||||||||||||
| import java.util.concurrent.atomic.AtomicInteger; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| public class BukkitTaskManager extends TaskManager { | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| private final Plugin plugin; | ||||||||||||||||||||||||||||||
| private final AtomicInteger foliaTaskCounter = new AtomicInteger(); | ||||||||||||||||||||||||||||||
| private final Map<Integer, Runnable> foliaTaskCancels = new ConcurrentHashMap<>(); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| public BukkitTaskManager(final Plugin plugin) { | ||||||||||||||||||||||||||||||
| this.plugin = plugin; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||
| public int repeat(@Nonnull final Runnable runnable, final int interval) { | ||||||||||||||||||||||||||||||
| return this.plugin.getServer().getScheduler().scheduleSyncRepeatingTask(this.plugin, runnable, interval, interval); | ||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||
| return this.plugin.getServer().getScheduler().scheduleSyncRepeatingTask(this.plugin, runnable, interval, interval); | ||||||||||||||||||||||||||||||
| } catch (UnsupportedOperationException ignored) { | ||||||||||||||||||||||||||||||
| return scheduleFoliaRepeatingTask(runnable, interval); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||
|
|
@@ -31,12 +42,20 @@ public void async(@Nonnull final Runnable runnable) { | |||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||
| public void task(@Nonnull final Runnable runnable) { | ||||||||||||||||||||||||||||||
| this.plugin.getServer().getScheduler().runTask(this.plugin, runnable).getTaskId(); | ||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||
| this.plugin.getServer().getScheduler().runTask(this.plugin, runnable).getTaskId(); | ||||||||||||||||||||||||||||||
| } catch (UnsupportedOperationException ignored) { | ||||||||||||||||||||||||||||||
| executeFoliaNow(runnable); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||
| public void later(@Nonnull final Runnable runnable, final int delay) { | ||||||||||||||||||||||||||||||
| this.plugin.getServer().getScheduler().runTaskLater(this.plugin, runnable, delay).getTaskId(); | ||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||
| this.plugin.getServer().getScheduler().runTaskLater(this.plugin, runnable, delay).getTaskId(); | ||||||||||||||||||||||||||||||
| } catch (UnsupportedOperationException ignored) { | ||||||||||||||||||||||||||||||
| scheduleFoliaDelayedTask(runnable, delay); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||
|
|
@@ -47,8 +66,85 @@ public void laterAsync(@Nonnull final Runnable runnable, final int delay) { | |||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||
| public void cancel(final int task) { | ||||||||||||||||||||||||||||||
| if (task != -1) { | ||||||||||||||||||||||||||||||
| Bukkit.getScheduler().cancelTask(task); | ||||||||||||||||||||||||||||||
| Runnable foliaCancel = foliaTaskCancels.remove(task); | ||||||||||||||||||||||||||||||
| if (foliaCancel != null) { | ||||||||||||||||||||||||||||||
| foliaCancel.run(); | ||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||
| Bukkit.getScheduler().cancelTask(task); | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
| Bukkit.getScheduler().cancelTask(task); | |
| try { | |
| Bukkit.getScheduler().cancelTask(task); | |
| } catch (UnsupportedOperationException ignored) { | |
| // Environment (e.g., Folia) does not support Bukkit scheduler cancel | |
| } |
Copilot
AI
Feb 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The scheduleFoliaDelayedTask method doesn't return the task ID from storeFoliaTaskCancel, but the later method's return signature expects an int. This means delayed Folia tasks cannot be cancelled because no task ID is returned to the caller. The storeFoliaTaskCancel call on line 94 should have its return value returned from this method.
| private void scheduleFoliaDelayedTask(final Runnable runnable, final long delay) { | |
| try { | |
| Object globalRegionScheduler = getGlobalRegionScheduler(); | |
| Method runDelayed = globalRegionScheduler.getClass() | |
| .getMethod("runDelayed", Plugin.class, java.util.function.Consumer.class, long.class); | |
| Object scheduledTask = runDelayed.invoke(globalRegionScheduler, this.plugin, new FoliaTaskConsumer(runnable), delay); | |
| storeFoliaTaskCancel(scheduledTask); | |
| private int scheduleFoliaDelayedTask(final Runnable runnable, final long delay) { | |
| try { | |
| Object globalRegionScheduler = getGlobalRegionScheduler(); | |
| Method runDelayed = globalRegionScheduler.getClass() | |
| .getMethod("runDelayed", Plugin.class, java.util.function.Consumer.class, long.class); | |
| Object scheduledTask = runDelayed.invoke(globalRegionScheduler, this.plugin, new FoliaTaskConsumer(runnable), delay); | |
| return storeFoliaTaskCancel(scheduledTask); |
Copilot
AI
Feb 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The scheduleFoliaDelayedTask and scheduleFoliaRepeatingTask methods accept parameters typed as long (delay, interval), but the calling methods (later, repeat) pass int parameters. While Java will auto-widen int to long, this type inconsistency could cause confusion. Consider keeping the parameter types consistent with the parent class method signatures (int) or explicitly casting at the call site to document the conversion.
Copilot
AI
Feb 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The RuntimeException thrown when cancelling a Folia task fails will propagate to the caller of cancel(), potentially causing unexpected crashes. Since cancel() is typically called during cleanup operations (like in onDisable), this could prevent proper shutdown. Consider logging the error instead of throwing, or wrapping it in a try-catch block at the call site to ensure cleanup continues even if cancellation fails.
Copilot
AI
Feb 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reflection methods (getMethod and invoke) are called on every task schedule operation, which could impact performance for frequently scheduled tasks. Consider caching the Method objects (execute, runDelayed, runAtFixedRate, cancel) as class fields after the first successful lookup to avoid repeated reflection overhead.
Copilot
AI
Feb 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The foliaTaskCancels map can accumulate entries for tasks that complete naturally without being explicitly cancelled. When a Folia task finishes on its own (non-repeating tasks), the cancel handler remains in the map indefinitely, causing a memory leak. Consider having the FoliaTaskConsumer remove its entry from foliaTaskCancels after the runnable completes, or implement a cleanup mechanism for completed tasks.
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -457,7 +457,11 @@ public void onDisable() { | |||||||||
| if (config != null) { | ||||||||||
| config.unload(); | ||||||||||
| } | ||||||||||
| this.getServer().getScheduler().cancelTasks(this); | ||||||||||
| try { | ||||||||||
| this.getServer().getScheduler().cancelTasks(this); | ||||||||||
| } catch (UnsupportedOperationException ignored) { | ||||||||||
| // Folia does not support legacy scheduler cancellation for all task types. | ||||||||||
|
||||||||||
| // Folia does not support legacy scheduler cancellation for all task types. | |
| // Folia does not support legacy scheduler cancellation for all task types. | |
| LOGGER.warn("Legacy Bukkit scheduler task cancellation is not supported on this server (likely Folia). " | |
| + "Any tasks scheduled via the legacy Bukkit scheduler may not have been cancelled during shutdown."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Starting the AtomicInteger at 0 and decrementing means the first Folia task will get ID -1, which is used as a sentinel value meaning "no task" in the cancel method (line 68). This will cause the first Folia task to be uncancellable. Consider initializing foliaTaskCounter with a value like Integer.MIN_VALUE + 1 to ensure task IDs stay far away from -1, or use incrementAndGet() starting from a large negative base to generate unique negative IDs that don't conflict with -1.