Chunks refusing to unload due to incorrect player ticket additions

I've received many reports of chunks refusing to unload and I have tracked this issue to vanilla. I have NOT been able to test on the latest pre-releases, but inspection of mapped source code shows the issue still potentially occurs. I have however tested on mcp-mapped sources and confirmed this issue for 1.14.2.

In order to reproduce, follow these steps:

1. Move around quickly (elytra + firework rocket)
2. Disconnect while moving with the elytra and firework rocket

This has the potential to add player tickets after a player has left.

It is HIGHLY recommended you test on a server that is struggling in terms of TPS. The issue that causes player tickets to be added is a race condition, and servers ticking less often will make this race condition easier to reproduce. I've personally found on servers not struggling that the issue is very difficult to reproduce. I've found running the server through a VM is good enough.

 

The issue here is that we disconnect the player while a player ticket is queued to potentially be added (explanation of code is below), and the elytra + firework only make that easier to do (more movement of tickets).

 

As for code inspection:

I am using MCP mappings to describe code.

The focus is in the net.minecraft.world.chunk.TicketManager class, specifically the func_219353_a() function, pasted below:

https://gist.github.com/Spottedleaf/cfe7882ab401fca339245cd28f8423c3

I've also labeled important sections with //, which I will be referencing.

 

Each server tick, the func_219353_a() function is called. This leads to this function being executed (from line 3): https://gist.github.com/Spottedleaf/cfe7882ab401fca339245cd28f8423c3#file-ticketmanager-java-L74

 

The called function appears to be the code handling player ticket movement. The function at line 51 of the gist appears to be the function which handles a change of state of a ticket. The boolean parameters relate to whether a chunk is out of range.

 

Following that, a ticket addition is queued if the chunk is now in range and previously was not. This is exactly where the issue occurs; if a player has left before the queued task is executed, the player's ticket will still be registered once the queued task eventually executes. The chunks are not checked if they're of range since the player has left, so the ticket will remain until the player joins the server again and leaves (potentially at least).

 

Finally, logging in to a different location (whether by alternate account or by editing my player file) shows that these tickets are still retained. As such this has the potential to load several thousands of chunks for larger servers (and I've had reports of 40k).

I understand these tickets will not tick and as such may not have a large impact for TPS, however they still hit hard in terms of RAM usage (remember, one player ticket can keep 700+ FULL loaded chunks) and auto saving thousands of chunks hits hard too. Eventually abusive players can kill a server if they load enough chunks due to RAM usage.

The steps to produce are quite easy, there are only two. Players with many accounts can perform this at separate locations to load many chunks. As such, this is labeled a private ticket.

 

UPDATE:

After further debugging I have found the issue to be an ordering issue. A ticket remove is scheduled for a chunk location pending a ticket addition, and the ticket remove is executed first. I have a fix located here - https://github.com/PaperMC/Paper/pull/2195