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

Fast server switch causes "race condition" in client #1334

Open
valaphee opened this issue May 27, 2024 · 7 comments
Open

Fast server switch causes "race condition" in client #1334

valaphee opened this issue May 27, 2024 · 7 comments

Comments

@valaphee
Copy link

valaphee commented May 27, 2024

Due to chat signing, after a server switch / join packet the client spins up a future which fetches the player session and sends it to the server.

Sometimes server do multiple switches closely together (but still after the first one in Velocities view is finished), and if the player has a slower/unstable connection, the player session packet is sent during the config phase which causes an exception and results in a disconnect.

Contents of the log
image

Culprit in the client

if (this.connection.isEncrypted()) {
    this.client.getProfileKeys().fetchKeyPair().thenAcceptAsync(keyPair -> keyPair.ifPresent(this::updateKeyPair), (Executor)((Object)this.client));
}

To circumvent this it might be necessary to wait for the packet to arrive with a timeout (3 seconds should be enough)

@electronicboy
Copy link
Member

electronicboy commented May 27, 2024

I'm not fond of the notion of solving these sort of issues on our side because it starts to introduce a whole bunch of extra state that we need to manage which gets fun because of how many broken clients exist, especially around this system...

Though, I guess that that would be a fair hackaround for this solution to try to minimise what is happening. I really do want to rewrite a whole bunch of the server transfer stuff, but, it's a fair while aways before I can really commit to taking on such a project

I want to say that there is already a mojira ticket for this issue?

@valaphee
Copy link
Author

valaphee commented May 27, 2024

Fair, for completeness https://bugs.mojang.com/browse/MC-272506, but it's not that common that they fix 3rd party issues even though they introduced a lot of stuff for 3rd partys recently.

And this wouldn't help for versions prior the version where it might get fixed.

@electronicboy
Copy link
Member

electronicboy commented May 27, 2024

This is a race condition regarding a system they implemented, it's likely not a priority for them, but, it is on their side; We'd need to see if we can mitigate it reasonably for affected versions, but reducing the amount of hackarounds in effect for any version is always nice

@U5B
Copy link
Contributor

U5B commented Aug 8, 2024

Looks like they recently resolved it for 1.21.1
https://bugs.mojang.com/browse/MC-272506

@Timongcraft
Copy link
Contributor

Looks like they recently resolved it for 1.21.1 https://bugs.mojang.com/browse/MC-272506

Will probably be 1.21.2, as 1.21.1 is a hotfix that fixes a crash bug.

@Gerrygames
Copy link
Contributor

1.21.1 does not contain the fix for this

@Timongcraft
Copy link
Contributor

1.21.1 does not contain the fix for this

That's what I said.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants