Assign ServerConnection to Player before processing post-login packets
Currently, when transitioning to a new server, the proxy begins auto-reading (and therefore processing) packets from the pending server connection before assigning that connection to the player via ConnectedPlayer::setConnectedServer. This causes a race condition if the backend server sends certain packets (particularly plugin messages) right after sending a JoinGamePacket. For example, the following has inconsistent behavior depending on when a plugin message is sent:
@Subscribe
public void onPluginMessage(PluginMessageEvent event) {
if(event.getSource() instanceof ServerConnection sc) {
// Will work consistently
sc.sendPluginMessage(MinecraftChannelIdentifier.create("test", "message"), "Hello, World".getBytes());
// Will often fail if the backend server sends a plugin message immediately after join.
sc.getPlayer()
.getCurrentServer()
.orElseThrow()
.sendPluginMessage(MinecraftChannelIdentifier.create("test", "message"), "Hello, World".getBytes());
}
}
This PR reverses the order of two lines in TransitionSessionHandler, making the proxy wait to process packets sent by a backend server until after assigning the player's connection.
Pretty sure that this does not make a difference. The code in question runs on the event loop of the server connection, so no packets from that connection can be handled during its execution.
Has this been tested? Because I'd imagine that the more bigger issue here is going to be that disabling auto-read doesn't promise to disable reading everything, and so there can still be stuff which ends up being processed, at least that is my understanding; The auto-reading re-enable + field update both occur within the same area.
Has this been tested?
I created this PR because I ran into errors caused by this bug while working on a plugin for my private server, but just to be sure, I spent some time writing a plugin to test it. I tested with both Velocity build 385, downloaded from the website, and my own fork. I tested both Spigot 1.20.5 and Fabric 1.20.5 backends. I got about a 40% error rate when using build 385, and a 0% error rate using my fork. I have attached the relevant logs and code.