Apply Folia inventory rollbacks on each player's scheduler - #1018
Merged
Intelli merged 4 commits intoOct 2, 2026
Merged
Conversation
Inventory rollbacks run in the region that owns chunk 0,0 and changed every player's inventory from there, which Folia does not allow. On Folia the rows are now grouped per player and applied on that player's entity scheduler, the batch waits for those tasks, and the rollback counters are updated atomically. Paper and Spigot are unchanged.
❌ Deploy Preview for coreprotect failed. Why did it fail? →
|
Contributor
|
Thanks -- automated review is requesting the following changes:
|
The per-player inventory tasks now go through the rollback's entity context. A task checks for cancellation before it changes anything, runs as an active mutation so cleanup waits for it to finish, and adds its item count to that context instead of the shared counter for the sender, so a late task cannot change a rollback that already ended or a newer one from the same sender. The counter helpers go back to their upstream form since player tasks no longer write to them.
…a-inventory-rollback
…a-inventory-rollback
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
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.
Summary
On Folia,
/co rollback ... #inventorychanges player inventories from the region thread that owns chunk 0,0, not from the thread that owns the player. This change moves those inventory writes onto each player's entity scheduler and makes the existing per-batch wait cover them. Paper and Spigot keep the inline path unchanged.The problem
An inventory rollback sets
inventoryRollbackand gives every row chunk key 0 (Rollback.java:244), so all rows for a world land in one "chunk" at 0,0. On Folia,processFoliaChunksruns each batch throughscheduleFoliaChunkBatchTask, which schedules the work withScheduler.scheduleSyncDelayedTask(..., chunkLocation, 0)(Rollback.java:610). That is the region thread that owns chunk 0,0 in the target world.RollbackProcessor.processChunkthen applies each row inline:player.getEnderChest()andplayer.getInventory()are modified atRollbackProcessor.java:283-285, and armor slots are re-sorted at line 373. Players are almost never in the region that owns chunk 0,0. Folia requires a player's state to be touched only from the thread that ticks that player, so this write is not allowed on Folia and it races the player's own tick (item pickup, inventory clicks, the player's save).Scenario: a griefer steals items and logs off, an admin at spawn runs
/co rollback u:Griefer t:1h #inventory, and the victim is online 3000 blocks away. The spawn region's thread writes the victim's inventory while the victim's own region is ticking them. The result is undefined: items can go missing, or reappear after the player's own tick overwrites the change.The fix
Rollback.javaprocessChunkWorld: on Folia it creates a list, passes it toRollbackProcessor.processChunk, and hands every future in it toentitySpawnContext.addPending(...). Upstream's loop already callsawaitChunkTasks(entitySpawnContext.drainPending(), preview)after each batch, so the next batch and the final count read at line 387 wait for the player tasks. On Paper and Spigot it passesnull.prepareChunkCountersandcompleteChunkwriterollbackHashwithcomputeIfPresentinstead ofgetthenput. The item count is now added from the player's thread while the region thread may still be in these methods for the same batch. Withgetthenput, either side could overwrite the other's update and the admin would see a wrong item total.rollbackHashis aCollections.synchronizedMap, whosecomputeIfPresentholds the map lock for the whole update.RollbackProcessor.javaprocessChunktakes aList<CompletableFuture<Boolean>> inventoryTasks. When it is not null and this is an inventory rollback, rows are grouped per player in aLinkedHashMap(row order per player is kept) instead of being applied.PaperAdapter.ADAPTER.executeEntityTask. The task applies that player's rows in order, re-sorts armor slots like the inline path, adds the item count, then completes the future.applyInventoryRowso both paths share it. It passesnullas the container type. That is what the inline path already passed:containerTypeis only assigned in the container branch, which is skipped for the whole call wheninventoryRollbackis true.updateRollbackHashusescomputeIfPresent, andaddRollbackItemsis the item-only atomic update the player task uses. Both use upstream's existing key, the user string.Offline and retired players: the inline path skips a row when
getPlayer(uuid)returns null. If the player logs out after grouping, Folia runs the retired callback orexecutereturns false. Both complete the future withtrueand add no items, so the player is skipped the same way and the total the admin sees counts only applied items.Failure: if the player task throws, it reports the error, adds the items applied so far, and completes
false. The batch wait then returns false, the loop logsROLLBACK_ABORTEDand cancels, and the sender gets the aborted message. On the inline path, an exception sets status 2, which also aborts.Alternatives I rejected:
RollbackProcessor. A plain list keepsRollbackProcessorfree of the context type and leaves the Bukkit path atnull.prepareChunkCountersresets that slot before each chunk, so the flag could be lost. The future result is not lost.Behaviour change
Risk
rollbackHashwriter that can run during a Folia batch is now atomic. The status check incompleteChunkis still a read followed by a write. Only the region and rollback threads set status 2, and the player task never does, so that check is not racing the new code.awaitChunkTasksexpires, a player task can still run afterwards. This is the same as upstream's entity spawn tasks. The rows were already flagged as rolled back before any world work started, so applying them late matches the database.processChunkclears, soitemData.clear()does not affect them.executeEntityTaskreturns false off Folia and is only called when the list is non-null, which is Folia only.Testing
Build:
mvn packagepasses.Row parity: the 47-step scenario (with a block rollback and restore) on Paper 26.2 and Folia 1.21.11 with SQLite matches upstream apart from the random plant that bone meal grows, with no new errors, and the world fingerprints after its rollback and restore steps differ from upstream's only in that same plant and in loose item entities (zombie drops, which are random). The scenario has no
#inventoryrollback step, so this shows the shared rollback code is unaffected, not the inventory path itself.Suggested live test on Folia with two players far apart (for example spawn and 3000 blocks out):
/co rollback u:B t:5m #inventory, then/co restore u:B t:5m #inventory.