reset pending keepalive state on listener handoff during reconfiguration#13712
Open
ZECHEESELORD wants to merge 2 commits intoPaperMC:mainfrom
Open
reset pending keepalive state on listener handoff during reconfiguration#13712ZECHEESELORD wants to merge 2 commits intoPaperMC:mainfrom
ZECHEESELORD wants to merge 2 commits intoPaperMC:mainfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes PaperMC/Velocity#1723. (The backend side)
During mid session reconfiguration behind Velocity, Paper was carrying the same
KeepAlivetracker across listener handoff. That allowed a pending PLAY keepalive expectation to survive into the CONFIG listener.In the failing case, the sequence was:
AABBarrived while the backend still consideredAoutstandingBas stale or out-of-order and disconnected the player withdisconnect.timeoutThe key signal from backend logging was that
listener_handoffshowed the CONFIG listener inheriting the prior PLAYexpectedId, and the next CONFIG keepalive reply was then rejected asstale_or_out_of_order.So the failure was not that Velocity dropped the CONFIG reply (the backend received it). The problem was that stale pending PLAY keepalive state leaked across the PLAY -> CONFIG handoff.
the fix:
ServerCommonPacketListenerImpl#createCookie(...)now gives the next listener a freshKeepAliveinstance instead of reusing the current listener's pending tracker. This preserves latency/history state, but intentionally resets inflight keepalive challenge state across listener handoff. In other words:That restores the expected protocol boundary between PLAY and CONFIG.
testing:
Tested locally with the Velocity reproduction from PaperMC/Velocity#1723.
Before this fix:
After this fix: