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

Fix chat race condition #1042

Merged

Conversation

EpicPlayerA10
Copy link
Contributor

@EpicPlayerA10 EpicPlayerA10 commented Jul 11, 2023

The issue

Chat system in velocity has a race condition. When player is sending messages very fast, then a backend server will kick him for out-of-order chat.

You can test this by installing litematica mod and use the feature called Paste schematic into world. Then litematica will send commands very fast and then you get kicked from the proxy. This issue only happens when player is connected to the proxy. When you are connected to the minecraft server directly then the issue disappears.

The race condition looks like this:

Flowchart before

Flowchart before

You can see here that velocity doesn't wait for packet to be sent. The chat gets out of sync and thats why players are getting kicked from backend server.

Fix

This pull request fixes it by waiting for packets to be sent. Now the flowchart looks like this:

Flowchart after

Flowchart after

Related issues

@electronicboy
Copy link
Member

Don't have hardware to look into this properly, but on the surface, this looks much closer to how I said this stuff should probably work in the first place

@intersecato
Copy link

intersecato commented Aug 21, 2023

Hi @EpicPlayerA10 @electronicboy,
we were able to test the fix in production with various users. The problem seems to persists but more rarely.
We will try to investigate the problem further.

image

@EpicPlayerA10
Copy link
Contributor Author

The problem seems to persists but more rarely.

Okay, now the latest commit will ensure that chat packets are fully sent. This should get rid of this problem.

MattiaFioretti added a commit to CoralRP/Velocifix that referenced this pull request Aug 28, 2023
@intersecato
Copy link

intersecato commented Aug 29, 2023

Okay, now the latest commit will ensure that chat packets are fully sent. This should get rid of this problem.

The problem is still there. A player reported me about a kick again today.

@EpicPlayerA10
Copy link
Contributor Author

The problem is still there. A player reported me about a kick again today.

Hmm. I don't know why this is still happening. I have no idea. I think that i did everything correct.

@electronicboy electronicboy self-assigned this Oct 12, 2023
@CatTeaA
Copy link

CatTeaA commented Jan 4, 2024

Is there any progress? Looking forward to it

@electronicboy
Copy link
Member

I recall looking over this a few times and it generally looked fine, finally have hardware to test, just time; if this can be rebased, I'll try and get to this on the weekend or sometime next week

@EpicPlayerA10
Copy link
Contributor Author

Rebased

@electronicboy electronicboy merged commit 08c03aa into PaperMC:dev/3.0.0 Jan 11, 2024
1 check passed
@4drian3d 4drian3d added this to the Velocity 3.3.0 milestone Feb 19, 2024
@ElementalMC
Copy link

this issue still exists on 1.20 paper backend server on client version 1.21. Im using the most recent version of velocity.
It only happens with commands though.
image

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

Successfully merging this pull request may close these issues.

6 participants