Skip to content
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

FAWE potentially writing to ChunkSection PalettedContainer whilst data is read for packet sending #2729

Open
2 tasks done
WyndevCodes opened this issue May 18, 2024 · 5 comments
Labels
Approved Added if an issue has been approved by a maintainer Bug Something isn't working

Comments

@WyndevCodes
Copy link

Server Implementation

Paper

Server Version

1.20

Describe the bug

I run a dungeon server that generates dungeons and then teleports players into those dungeons. The players are teleported from the spawn world into the dungeon world after the schematics are loaded.

The issue is that I get an error almost every time a dungeon is generated (see the stack trace below). It doesn't seem to do anything normally, but it may result rarely in parts of the dungeon not being pasted properly.

Here is the code I use to paste the schematics (this runs about 16 times in 1-5 ticks on an async thread):

public boolean pasteSchematic(String schematicName, Location loc, Main main) {

        World weWorld = FaweAPI.getWorld(loc.getWorld().getName());
        File file = new File(main.getDataFolder() + File.separator + "schematics" + File.separator + schematicName + ".schem");
        if (!file.exists()) {
            Bukkit.getLogger().warning("The dungeon room file " + schematicName + ".schem does not exist in the DungeonDodge schematics folder!");
            return false;
        }
        //Bukkit.getLogger().info("[Dungeon Generation] Dungeon room instance has schematic " + schematicName + ".schem");

        BlockVector3 vec = BlockVector3.at(loc.getBlockX(), loc.getBlockY(), loc.getBlockZ());
        try {
            ClipboardFormat clipboardFormat = ClipboardFormats.findByFile(file);
            if (clipboardFormat != null) {
                Clipboard clipboard = clipboardFormat.load(file);
                EditSession editSession = clipboard.paste(weWorld, vec);
                clipboard.close();
                editSession.close();
            }
        } catch (IOException e) {
            Bukkit.getLogger().severe("Schematic " + schematicName + ".schem was not pasted properly!");
            e.printStackTrace();
        }
        return true;
    }

To Reproduce

  1. Create a piece of code that pastes schematics in a sequential form inside of a minecraft world different from the one a player resides in (e.g. paste a hollow square 16 times, adding the square's length to the z-axis each time). This code should be run async and via a command.
  2. Teleport the player inside of the first square pasted.
  3. Check console for any errors.

Expected behaviour

There should be no error when sending pasted chunk packets to players.

Screenshots / Videos

No response

Error log (if applicable)

https://paste.gg/p/anonymous/9edac91f450e4000b82844d086674fa4

Fawe Debugpaste

https://athion.net/ISPaster/paste/view/00996f5b6b484f6484fd3ab4a9542fba

Fawe Version

FastAsyncWorldEdit-638

Checklist

Anything else?

I am not using the newest version but had this issue before the recent update. I looked through the patch notes on the recent update and saw no fix for this issue there.

@WyndevCodes WyndevCodes added the Requires Testing This is a new issue which needs to be approved before labeled with "bug" label May 18, 2024
@WyndevCodes
Copy link
Author

Note that the server version is 1.20.2 and I cannot upgrade/downgrade due to plugin and world restrictions. In the case that it is found that this is a minecraft chunk loading issue, I will close this thread.

@PierreSchwang
Copy link
Member

More information including the potential cause as identified by the paper team over at #2652

@PierreSchwang PierreSchwang added Bug Something isn't working Approved Added if an issue has been approved by a maintainer and removed Requires Testing This is a new issue which needs to be approved before labeled with "bug" labels May 25, 2024
@dordsor21
Copy link
Member

Looking into this a bit, I believe it could be caused by how we write biomes directly to the palette container, rather than making a copy and directly replacing it like with blocks

dordsor21 added a commit that referenced this issue Jun 2, 2024
 - Removes possibility of writing to the LevelChunkSection biomes PalettedContainer whilst it is being read for sending packets
 - I believe this occured mostly on clipboard operations where blocks are written before biomes, so chunks are being sent whilst writing biomes
 - This would explain why the error reported in the below issue (and others) is/was so rare
 - Of course I could be completely wrong about all of this, but given the line in LevelChunkSection#write that the error seems to consistently occur on is when writing biomes to the packet, and that the only place I can find in FAWE where we write to a "live" PalettedContainer is for biomes, I am reasonably confident that this is the cause
 - Should address #2729
@dordsor21 dordsor21 changed the title Error generated in console when sending chunk packets to users after pasting a set of schematics FAWE potentially writing to ChunkSection PalettedContainer whilst data is read for packet sending Jun 2, 2024
NotMyFault pushed a commit that referenced this issue Jun 15, 2024
* fix: improve biome setting to avoid writing directly to chunk

 - Removes possibility of writing to the LevelChunkSection biomes PalettedContainer whilst it is being read for sending packets
 - I believe this occured mostly on clipboard operations where blocks are written before biomes, so chunks are being sent whilst writing biomes
 - This would explain why the error reported in the below issue (and others) is/was so rare
 - Of course I could be completely wrong about all of this, but given the line in LevelChunkSection#write that the error seems to consistently occur on is when writing biomes to the packet, and that the only place I can find in FAWE where we write to a "live" PalettedContainer is for biomes, I am reasonably confident that this is the cause
 - Should address #2729

* Remove self-refraction-check
@SecretlyJealous
Copy link

I'm not sure if the fix has been implemented yet but I recently got this crash again using build 850: https://pastebin.com/easELqEA

@dordsor21
Copy link
Member

FAWE itself should no longer be writing to the chunk section whilst creating a packet, but there is not guarantee that the server is not doing either of the above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Added if an issue has been approved by a maintainer Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants