Repository navigation
fix(dialog): keep the pre-join dialog open when the timeout is 0 - #3164
Merged
Merged
Conversation
loginTimeout and registerTimeout are documented as "Set to 0 to disable" and the post-join timeout task honours that. The pre-join dialog clamped the value to at least one second instead, so with 0 the dialog closed after one second and the player was kicked with the timeout message. Only schedule the timeout when the setting is positive. Two things that relied on the timeout always firing are adjusted for the unlimited wait: - the configuration thread waiting for the answer is released when the connection closes, otherwise it would stay parked for good - the pending name claim no longer expires 60 seconds into the dialog, which let a second connection take over the name and got the first one refused as already online once it answered Fixes AuthMe#3122
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.
loginTimeout: 0andregisterTimeout: 0are documented as "Set to 0 to disable", andLimboPlayerTaskManagerhonours that for the post-join flow. The pre-join dialog doesMath.max(timeout, 1)beforecompleteOnTimeout, so 0 becomes one second: the dialog closes a second after it opens and the player is kicked with "Login timeout exceeded" (#3122). The clamp has been there since the dialog was added; I could not find a reason for it other than keepingcompleteOnTimeoutfrom firing immediately.With this change the timeout is only scheduled when the setting is positive. The server itself does not limit how long a client stays in the configuration phase as long as it answers keep-alives (the 30 second limit is in the login phase only), so the dialog can stay open. Two other places assumed the timeout always fires, and needed a change for that to be safe:
join()until the dialog is answered. When the player disconnected,retireSessiondropped the future without completing it, which was fine while the timeout woke the thread up a moment later. Without a timeout the thread stayed parked forever, soretireSessionnow completes the pending future.PaperLoginValidationListener.getClaimTtlMillis()returns the timeout plus 60 seconds, so with 0 the name claim went stale 60 seconds into the dialog. A second connection with the same name then got its own dialog, and the first player was refused as "already online" once they answered. When either timeout is disabled the claim is now held for a day; it still ends as soon as the connection closes. I picked a day so the claim still cannot lock a name out forever. Happy to change the value or the approach.One thing to be aware of: with the timeouts disabled, each waiting connection keeps one configuration thread until it answers or disconnects, and those connections do not count against max-players. That is what the setting asks for, but it is new for the pre-join phase.
Testing:
PaperDialogFlowListenerTest: the dialog is not ended after one second with a timeout of 0 (login and register), a timeout of 1 still kicks, and closing the connection completes its pending future.PaperLoginValidationListenerTest: the claim lifetime with a disabled timeout. The new assertions fail on master.mvn -V -B clean packageon JDK 21: build passes.custom_click_action, timeouts set to 0:PaperDialogFlowListener(one stayed parked when I only removed the clamp).loginTimeout: 3/registerTimeout: 3the player is still kicked after 3 s.Not tested: a real client clicking the dialog (the test client speaks the protocol directly), proxy setups, email registration.
Fixes #3122