Handle Folia scheduler UnsupportedOperationException in Bukkit task flow#2
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds Folia scheduler compatibility to FastAsyncWorldEdit (FAWE) by implementing fallback mechanisms for legacy Bukkit scheduling APIs that throw UnsupportedOperationException on Folia-based servers. Folia's global region scheduler requires different APIs than classic Paper/Spigot schedulers, and this change ensures FAWE can operate on both platforms without crashes during plugin lifecycle events.
Changes:
- Added try-catch wrappers around legacy Bukkit scheduler calls in
BukkitTaskManagerwith reflection-based Folia fallbacks - Implemented task tracking for Folia-scheduled tasks using
AtomicIntegerandConcurrentHashMapto enable cancellation - Protected
WorldEditPlugin.onDisable()from crashes when callingcancelTasks()on Folia
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| worldedit-bukkit/src/main/java/com/fastasyncworldedit/bukkit/util/BukkitTaskManager.java | Adds Folia scheduler support via reflection with task tracking and cancellation for repeat(), task(), and later() methods |
| worldedit-bukkit/src/main/java/com/sk89q/worldedit/bukkit/WorldEditPlugin.java | Wraps cancelTasks() call in try-catch to prevent disable-time crashes on Folia |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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); | ||
| } catch (ReflectiveOperationException e) { | ||
| throw new RuntimeException("Unable to schedule delayed sync task on Folia", e); | ||
| } | ||
| } | ||
|
|
||
| private int scheduleFoliaRepeatingTask(final Runnable runnable, final long interval) { |
There was a problem hiding this comment.
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.
| public class BukkitTaskManager extends TaskManager { | ||
|
|
||
| private final Plugin plugin; | ||
| private final AtomicInteger foliaTaskCounter = new AtomicInteger(); |
There was a problem hiding this comment.
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.
| private final AtomicInteger foliaTaskCounter = new AtomicInteger(); | |
| private final AtomicInteger foliaTaskCounter = new AtomicInteger(Integer.MIN_VALUE + 1); |
| 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); |
There was a problem hiding this comment.
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); |
| try { | ||
| this.getServer().getScheduler().cancelTasks(this); | ||
| } catch (UnsupportedOperationException ignored) { | ||
| // Folia does not support legacy scheduler cancellation for all task types. |
There was a problem hiding this comment.
While catching UnsupportedOperationException here prevents crashes on Folia, silently ignoring the exception means that tasks scheduled through the legacy Bukkit scheduler won't be cancelled during plugin shutdown. This could lead to tasks continuing to run after the plugin is disabled. Consider logging a warning when this fallback is triggered, or implementing a Folia-compatible cancellation mechanism that tracks and cancels all tasks (both Bukkit and Folia) during shutdown.
| // 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."); |
| if (foliaCancel != null) { | ||
| foliaCancel.run(); | ||
| } else { | ||
| Bukkit.getScheduler().cancelTask(task); |
There was a problem hiding this comment.
The cancel method attempts to cancel a Bukkit task even when no Folia cancel handler exists, but it doesn't handle the UnsupportedOperationException that Bukkit.getScheduler().cancelTask() may throw on Folia. If a Bukkit task ID is passed but Folia is the active scheduler, this will throw an exception. Wrap the cancelTask call in a try-catch block to handle this scenario gracefully.
| Bukkit.getScheduler().cancelTask(task); | |
| try { | |
| Bukkit.getScheduler().cancelTask(task); | |
| } catch (UnsupportedOperationException ignored) { | |
| // Environment (e.g., Folia) does not support Bukkit scheduler cancel | |
| } |
| private record FoliaTaskConsumer(Runnable runnable) implements java.util.function.Consumer<Object> { | ||
|
|
||
| @Override | ||
| public void accept(final Object ignored) { | ||
| runnable.run(); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
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.
| } catch (ReflectiveOperationException e) { | ||
| throw new RuntimeException("Unable to cancel Folia task", e); | ||
| } |
There was a problem hiding this comment.
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.
| private void executeFoliaNow(final Runnable runnable) { | ||
| try { | ||
| Object globalRegionScheduler = getGlobalRegionScheduler(); | ||
| Method execute = globalRegionScheduler.getClass().getMethod("execute", Plugin.class, Runnable.class); | ||
| execute.invoke(globalRegionScheduler, this.plugin, runnable); | ||
| } catch (ReflectiveOperationException e) { | ||
| throw new RuntimeException("Unable to schedule sync task on Folia", e); | ||
| } | ||
| } | ||
|
|
||
| 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); | ||
| } catch (ReflectiveOperationException e) { | ||
| throw new RuntimeException("Unable to schedule delayed sync task on Folia", e); | ||
| } | ||
| } | ||
|
|
||
| private int scheduleFoliaRepeatingTask(final Runnable runnable, final long interval) { | ||
| try { | ||
| Object globalRegionScheduler = getGlobalRegionScheduler(); | ||
| Method runAtFixedRate = globalRegionScheduler.getClass() | ||
| .getMethod("runAtFixedRate", Plugin.class, java.util.function.Consumer.class, long.class, long.class); | ||
| Object scheduledTask = runAtFixedRate.invoke( | ||
| globalRegionScheduler, | ||
| this.plugin, | ||
| new FoliaTaskConsumer(runnable), | ||
| interval, | ||
| interval | ||
| ); | ||
| return storeFoliaTaskCancel(scheduledTask); | ||
| } catch (ReflectiveOperationException e) { | ||
| throw new RuntimeException("Unable to schedule repeating sync task on Folia", e); | ||
| } | ||
| } | ||
|
|
||
| private int storeFoliaTaskCancel(final Object scheduledTask) { | ||
| try { | ||
| Method cancel = scheduledTask.getClass().getMethod("cancel"); | ||
| int taskId = foliaTaskCounter.decrementAndGet(); | ||
| foliaTaskCancels.put(taskId, () -> { | ||
| try { | ||
| cancel.invoke(scheduledTask); | ||
| } catch (ReflectiveOperationException e) { | ||
| throw new RuntimeException("Unable to cancel Folia task", e); | ||
| } | ||
| }); | ||
| return taskId; | ||
| } catch (ReflectiveOperationException e) { | ||
| throw new RuntimeException("Unable to wire Folia task cancellation", e); | ||
| } | ||
| } | ||
|
|
||
| private Object getGlobalRegionScheduler() throws ReflectiveOperationException { | ||
| Server server = this.plugin.getServer(); | ||
| Method method = server.getClass().getMethod("getGlobalRegionScheduler"); | ||
| return method.invoke(server); | ||
| } |
There was a problem hiding this comment.
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.
Motivation
UnsupportedOperationExceptionfor legacy Bukkit scheduling APIs which caused FAWE to fail during plugin enable/disable on Folia-based servers.Description
BukkitTaskManagerby wrapping legacy sync scheduling calls (repeat,task,later) intry/catchand invoking Folia's scheduler via reflection whenUnsupportedOperationExceptionis thrown.AtomicIntegerand aConcurrentHashMapto track Folia-scheduled tasks and their cancellation handlers soTaskManager.cancel(int)works with both scheduler implementations.getGlobalRegionScheduler()on the server and useexecute,runDelayed, andrunAtFixedRate(via aFoliaTaskConsumerrecord) to run/track tasks on Folia, and wire up cancellation via the returned scheduled task'scancelmethod.WorldEditPlugin.onDisable()'scancelTasks(this)call with atry/catchforUnsupportedOperationExceptionto avoid disable-time crashes on Folia.Testing
./gradlew :worldedit-bukkit:compileJava --no-daemon, which exercised the modified code paths, but the build stopped due to an unrelated repository configuration issue:Task with name 'build' not found in project ':worldedit-libs:cli'(failure not caused by these changes).Codex Task