Add Folia support#6586
Open
Raw2d wants to merge 50 commits into
Open
Conversation
pending PaperMC/Folia#5 for global region helpers
# Conflicts: # EssentialsDiscord/src/main/java/net/essentialsx/discord/util/ConsoleInjector.java
# Conflicts: # Essentials/src/main/java/com/earth2me/essentials/commands/Commandskull.java
EconomyLayers relies on the scheduler to be initialized to enable properly
# Conflicts: # Essentials/src/main/java/com/earth2me/essentials/Essentials.java # Essentials/src/main/java/com/earth2me/essentials/commands/Commandessentials.java
Resolved 5 conflicted files per the Folia-first decision rule (see task-3 report). Needs Folia-logic re-check in a later phase: - Commandtime.java: ptime-preservation bugfix from 2.x reapplied inside the folia scheduleGlobalDelayedTask lambda; verify reading/writing per-player time state from within the global scheduled task is correct under Folia's threading model.
RoleSyncManager (EssentialsDiscordLink) declared its scheduled task as BukkitTask, but IEssentials#runTaskTimerAsynchronously on folia-support already returns SchedulingProvider.EssentialsTask. The 2.x merge pulled this class in without updating it to the Folia-side scheduler abstraction, causing a compile error. Switched the field type to SchedulingProvider.EssentialsTask, matching the existing pattern in EssentialsDiscord's DiscordCommandSender.
EssentialsDiscord's JDA/netty-thread interaction bridge (JDA gateway thread -> scheduleGlobalDelayedTask -> Bukkit.dispatchCommand) is not Folia-safe and was left broken by the prior refactor/folia WIP. Rather than attempt a fix, disable both EssentialsDiscord and EssentialsDiscordLink outright on Folia with a clear log message, and correct their plugin.yml folia-supported flags to false to match.
EssentialsXMPP is an optional external chat-bridge module, same category as Discord, and was not part of this port's core scope. Disable it on Folia with the same VersionUtil.FOLIA guard pattern used for EssentialsDiscord/EssentialsDiscordLink, and declare folia-supported: false explicitly.
TuneRunnable extended BukkitRunnable and was started via runTaskTimer(ess, 20, 2), a direct legacy Bukkit scheduler call that throws at runtime on Folia. Replaced with a plain TuneTicker driven by ess.scheduleGlobalRepeatingTask, and dispatched each online player's playSound call through ess.scheduleEntityDelayedTask so per-player work runs on that player's own region thread rather than the global thread that drives the tune.
Settings.reloadConfig() only wrapped its command-map mutations (_addAlternativeCommand, syncCommandsProvider.syncCommands) in scheduleGlobalDelayedTask during the plugin's first two startup reloads; every later /essentials reload ran them synchronously on whichever thread invoked the command, which on Folia is the executor's own region thread rather than the global thread these mutations require. Removed the reloadCount special-casing so both always schedule onto the global thread regardless of when the reload happens. A third, unrelated reloadCount.get() < 2 guard remains at line 847 (startup-only plugin re-registration logic that already unconditionally dispatches via scheduleGlobalDelayedTask within that branch) and was intentionally left untouched, as it is not the same-thread-unsafe defect this commit addresses.
Backup's own repeating task and its async-completion callback both run on Folia's single global region thread, so they don't race with each other. But Essentials.java's onDisable() reads getBackup().getTaskLock() during shutdown, which isn't guaranteed to run on that same thread. Plain fields gave no visibility guarantee for that read; marking running/task/active/taskLock volatile fixes the gap without needing to touch the scheduling design.
LocationUtil.getTarget ray-traces up to 300 blocks to find where the player is looking, then the command spawned a TNTPrimed entity at that location directly on the command-executing thread. A ray-traced point 300 blocks away is not guaranteed to be owned by that thread's current Folia region. Wrapped the spawn call in ess.scheduleLocationDelayedTask(loc, ...), the same primitive already used identically in Commandweather.java for a location-targeted world mutation.
… region The self-strike branch ran strikeLightning directly on the command thread against a block ray-traced up to 600 blocks away, with no dispatch at all -- inconsistent with the 'others' branch three lines below, which correctly wraps its strike in ess.scheduleEntityDelayedTask. Entity-dispatch isn't the right analog here though: the 'others' branch works because the target player's entity thread and the strike location coincide, but the self-strike target is an arbitrary distant block. Used ess.scheduleLocationDelayedTask(target, ...) instead, targeting the resolved strike location directly -- the same location-based primitive already used in Commandweather.java and Commandantioch.java.
Commandspawner ray-traced up to 300 blocks, then read and mutated the targeted spawner block's state directly on the command thread -- a Folia thread-affinity violation for any target not owned by that thread's current region. Wrapped the block-type check and the mutation in ess.ensureRegion(target, ...), the same blocking region-thread rendezvous already used via ensureEntity in AsyncTeleport.java. Since Runnable can't declare checked exceptions, capture the resulting TranslatableException in an AtomicReference and rethrow it on the calling thread after ensureRegion returns, so existing error-message behavior for players/console is preserved. As a side effect the block-type check now happens atomically with the mutation instead of as an earlier separate read, which changes error message priority when both the target block and the mob name are invalid (mob-name error now takes precedence) -- a minor UX ordering change, not a functional regression.
…ntityDelayedTask checkActivity(), called by EssentialsTimer from Folia's global thread, called this.getBase().kickPlayer(...) directly and looped over every online user calling user.sendTl(...) directly - both are entity mutations issued from the wrong thread on Folia. Wrap both in ess.scheduleEntityDelayedTask so each runs on its target player's own region thread, matching the same pattern Phase 2A's NyanCommand fix and the existing Commandlightning.java already use for per-player side effects inside a loop over online players.
calculateRandomLocation carried its own FIXME acknowledging that its PaperLib.getChunkAtAsync(...).thenApply(...) callback read/wrote block-derived state (getNetherYAt, getHighestBlockYAt) with no guarantee of running on the target location's owning region thread -- backing every /tpr invocation, not just cache misses. Replaced the thenApply transform with an explicit CompletableFuture completed from inside ess.scheduleLocationDelayedTask(location, ...), the same primitive used in Commandweather.java and Commandantioch.java. As a verified side effect, attemptRandomLocation's own unguarded location.getBlock().getBiome() read (via isExcludedBiome, not previously flagged) now also runs on the correct region thread, since its thenAccept continuation was already chained onto this future before completion and CompletableFuture.complete() runs already-attached synchronous continuations on the completing thread.
…stination's region thread nowAsync backs every teleport-driven command in the plugin (/tp, /tpa, /home, /warp, /spawn, /back, and via RandomTeleport /tpr). Its PaperLib.getChunkAtAsync(...).thenAccept(...) callback read block state via LocationUtil.isBlockUnsafeForUser and called PaperLib.teleportAsync with no guarantee the callback ran on the destination's owning Folia region thread. Wrapped the callback body in ess.scheduleLocationDelayedTask(targetLoc, ...), the same primitive used in Commandweather.java, Commandantioch.java, Commandlightning.java, and RandomTeleport.java earlier in this plan. LocationUtil.java itself required no change -- its isBlockUnsafe/isBlockAboveAir/isBlockDamaging methods are plain synchronous block readers with no scheduling logic; the bug was entirely in this call site not dispatching before invoking them.
Commandgc.java and KeywordReplacer.java both hardcoded tps = 20d with a TODO since commit 0e4bab2 deleted EssentialsTimer.getAverageTPS() and its supporting history/lastPoll state during the original Folia port. Restore the original self-timed measurement of EssentialsTimer's own global-repeating-task firing interval rather than building new per-region MSPT tracking: the per-region getTPS(Location) API that would require doesn't exist in this project's pinned folia-api:1.20.1-R0.1-SNAPSHOT dependency (confirmed via javap against that exact jar), and bumping that dependency is a separate, riskier change outside this plan's scope. The restored history buffer is now read cross-thread (by /tps and by chat placeholder resolution) where the original single-threaded plugin never had to worry about that, so recordTick()/getAverageTPS() are both synchronized to guard it. This reports the global region's own tick health, not a true whole-server aggregate across every per-player region - a known, documented limitation, not a hidden one.
…upported Discovered via live testing on a real Folia 1.21.8 server (not caught by any compile-check): Essentials.onDisable() called getServer().getScheduler() .cancelTasks(this) directly, which throws UnsupportedOperationException on Folia's CraftScheduler stub every time the plugin disables (including on every server stop/reload). Reproduced live, confirmed via stack trace at Essentials.java:606 -> CraftScheduler.cancelTasks -> CraftScheduler.handle. Added SchedulingProvider.cancelAllTasks(), implemented per-platform: BukkitSchedulingProvider delegates to the same legacy call (unchanged behavior off Folia); FoliaSchedulingProvider uses the Folia-safe GlobalRegionScheduler.cancelTasks(plugin) + AsyncScheduler.cancelTasks(plugin) (both verified to exist on the actual pinned folia-api jar via javap). Region-owned entity/location tasks don't need a separate cancel, since RegionShutdownThread's own halt sequence already tears those down during plugin disable. Essentials.java now calls the abstraction instead of the raw legacy scheduler.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on the old refactor/folia branch (PR #5291, stalled since April and 62 commits behind 2.x by the time I picked it up) rather than trying to land that PR as-is. Merged current 2.x into it as a single merge commit and went through the threading issues the original branch's own TODO list already flagged, plus a few more that turned up along the way.
What's fixed:
/tp,/tpa,/home,/warp,/spawn,/rtp) was reading block/chunk state and callingteleportAsyncinside an async chunk-load callback with no guarantee it runs on the destination region's thread. Dispatched through the region scheduler instead./antioch,/spawner, and/lightning's self-strike branch: block/entity mutation happening off the owning region's thread.EssentialsTimer(global thread) was mutatingUserfields that per-player region-thread event handlers also touch (lastActivity,teleportInvulnerabilityTimestamp, mute/jail timestamps) with no visibility guarantee, and callingkickPlayer()directly from the global thread./tpsand the{TPS}placeholder were hardcoded to20d. The tick timer got deleted at some point during the original Folia work and never replaced. Restored real measurement./nyan's tune player used a rawBukkitRunnable, which just throws on Folia.Settings.reloadConfig()only dispatched its command-map mutations onto the global thread during the first two startup reloads; every/essentials reloadafter that ran them on whatever thread called it.onDisable()calledgetServer().getScheduler().cancelTasks(this)directly, which throwsUnsupportedOperationExceptionon Folia's scheduler stub. Every plugin disable/reload was crashing. Added a scheduling-provider abstraction for it (Folia-safe path via the global region + async schedulers, unchanged legacy call on everything else).Tested on a real Folia 1.21.8 server, not just compile checks: enable/disable/reload cycles,
/tp//home//warp//spawn//rtp,/nyan, kit claims,/balanceand/eco,/tps, chat. No exceptions in any of it after the fixes above went in.