Skip to content

fix: avoid re-fetching connected server in handle(ClientSettingsPacket) - #1850

Open
qventymr wants to merge 1 commit into
PaperMC:dev/4.0.0from
qventymr:fix/client-settings-packet-race
Open

fix: avoid re-fetching connected server in handle(ClientSettingsPacket)#1850
qventymr wants to merge 1 commit into
PaperMC:dev/4.0.0from
qventymr:fix/client-settings-packet-race

Conversation

@qventymr

Copy link
Copy Markdown

What

ClientPlaySessionHandler#handle(ClientSettingsPacket) fetches
player.getConnectedServer(), null-checks it, then discards the local
variable and calls player.getConnectedServer() a second time before
calling .ensureConnected() on the result.

Between these two calls the player's connected server can change on
another thread (e.g. mid server-switch), so the second call can return
null (NPE on .ensureConnected()) or a connection whose
ensureConnected() throws IllegalStateException, crashing/kicking
the session — the same class of bug fixed for the chat/command packet
handlers in f6fbd25 ("Downgrade severity of handling several incoming
user input packet states"), which however did not touch this handler.

Fix

Reuse the already null-checked local serverConnection reference
instead of re-querying player.getConnectedServer().

Testing

Built locally with ./gradlew build; no behavioral change for the
non-racy path, since the second getConnectedServer() call previously
returned the same object in the common case.

@electronicboy

electronicboy commented Jul 24, 2026

Copy link
Copy Markdown
Member

I don't think this is an actual improvement outside of flow correction, this stuff, along with server transfer invocations, should occur on the event loop and so there should be no risk of something side-sweeping in here

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.

2 participants