FastAsyncWorldEdit

FastAsyncWorldEdit

152k Downloads

History (e.g. //undo) sometimes stops working completely when previous worlds are unloaded

Schuwi opened this issue ยท 2 comments

commented

Server Implementation

Paper

Server Version

1.18.2

Describe the bug

When using a plugin to manage multiple worlds (e.g. MultiVerse or BuildSystem) which unloads worlds when not in use, sporadically //undo stops working.
The circumstances to evoke this bug might seem a bit convoluted (see steps to reproduce) but the bug has occured multiple times in practice.

To Reproduce

  1. Setup a PaperMC 1.18.2 Server with FAWE and MultiVerse
  2. Create a new world e.g. "test" with /mv create test NORMAL -t FLAT (generation options are not relevant)
  3. Go into the newly generated world (/mv tp test)
  4. Do any worldedit operation that is saved in the history (e.g. //pos1, //pos2, //set stone)
  5. Go back to the default world and unload the test world (/mv tp world, /mv unload test)
  6. The problem now is that FAWE caches the world object with a WeakReference before trying to get the world by name via the Bukkit API so we need to:
    6.1. Either force garbage collection a few times for example with VisualVM, clutter the heap by flying around, loading chunks, wait a few minutes and pray to your favorite god(s)
    6.2. Or replace this line with World tmp = null; for testing
  7. Do another worldedit operation that impacts history (an exception should be thrown in the console but the player won't notice anything is wrong yet)
  8. Try to //undo this last operation. This will fail with an exception (java.lang.NullPointerException: The world was unloaded and the reference is unavailable) and the changes won't be undone.

Expected behaviour

No exceptions should be thrown and //undo should undo the latest changes.

Screenshots / Videos

No response

Error log (if applicable)

https://gist.github.com/Schuwi/072b8abd3b4861d21c89165963835c97

Fawe Debugpaste

https://athion.net/ISPaster/paste/view/a26d3e931462486c87a96737b4ade9f5

Fawe Version

FastAsyncWorldEdit version 2.1.1-SNAPSHOT-134;465c81d

Checklist

Anything else?

No response

commented

This currently works as intended, however, is technically a bug. Ultimately the decision needs to be made if we want to attempt to load an unloaded world, or if we want to skip history for this world (could be an issue for "lost" history in that world in the "future"), before implementing much of a fix

commented

Correct me if I'm wrong but as I understand the issue the world would neither have to be loaded nor would history be lost. I presume the history is recorded the moment that an operation is executed. Following the stack trace the only issue seems to be that FAWE wants to save a negativeHistoryIndex (don't ask me what it's for but it does not depend on the world being accessible) into a file that has the world name in the file name.

So the fix could be as simple as caching the world name or file name somewhere (the world name is actually already cached in BukkitWorld and returning worldNameRef in getName() [and getId for good measure I suppose] would fix the problem).
Another fix would be to call loadSessionHistoryFromDisk when the player switches worlds (PlayerChangedWorldEvent in bukkit). Then there should be no issues with deciding between loading world or lost history now or in the future (in case the current implementation of history saving changes) because history is already taken care of while the world is definitely still loaded (or maybe PlayerTeleportEvent would guarantee that, not sure).