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 race condition when handling unknown packets. #3508

Merged

Conversation

modmuss50
Copy link
Member

Fixes #3505

This was a fun one to figure out, this change ensures that custom payloads are always relaesed on the network thread, and no further processing of them happens on the main thread.

This caused a rare race codition where a user might be disconnected from a server sending unknown packets with the following error: io.netty.util.IllegalReferenceCountException: refCnt: 0

@modmuss50 modmuss50 added bug Something isn't working fabric-networking Pull requests and issues related to the networking api priority:high High priority PRs that need review and work now. Review these first. labels Jan 4, 2024
@modmuss50 modmuss50 requested review from deirn and a team January 4, 2024 23:12
@christopherplaysminecraft
Copy link

christopherplaysminecraft commented Jan 4, 2024

Thankful that it got fixed so fast, this has been annoying me for more than two weeks now.

I have found this playing BedWars on the Hypixel Server.
The server sends custom badlion:timers packets which allow the client to display information about the game (Time until new resources spawn, Game Events, etc.) and it does this even if your client doesn't even support those features.

This doesn't cause any issues, typically.

@modmuss50 modmuss50 added the merge me please Pull requests that are ready to merge label Jan 5, 2024
@modmuss50 modmuss50 merged commit 875cc14 into FabricMC:1.20.4 Jan 5, 2024
5 checks passed
modmuss50 added a commit that referenced this pull request Jan 5, 2024
modmuss50 added a commit that referenced this pull request Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fabric-networking Pull requests and issues related to the networking api merge me please Pull requests that are ready to merge priority:high High priority PRs that need review and work now. Review these first.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue with Fabric Networking API
3 participants