Repository navigation
Fix referees not being correctly dissociated from closed rooms in specific scenario - #543
Merged
Merged
Conversation
…cific scenario Should close ppy#541. The scenario here is as follows: 1. Referee A and B are in a room. (It does not matter which created the room / is host, or in what order they joined.) 2. Referee A drops connection to the hub, therefore leaving the room. Since ppy#466, this is no longer synonymous with losing referee privileges to the room - they are retained as spectator server memory state. 3. While referee A still has refereeing privileges but still has not rejoined the room, B disbands the room. 4. Referee A will now show as still associated to the room despite it being closed. The reason for this bug is an oversight. The room disband operation relied on iterating over `room.Users` to clean up all users' state. This was safe to do prior to ppy#466, because a referee drop-out would automatically incur a full leave including privilege revocation, but is not safe to do any more. To address this, change to a somewhat naive loop over all referees in the hub that ensures every single remaining one is dissociated from the closed room. I am not going to defend how it looks too hard, though extenuating circumstances here include: - this hub is concurrently used by double-digit clients at the absolute most at this time, - room closes should be relatively rare operations. If despite the above the fix is still considered too offensive to live, I will attempt to conjure some sort of inverse memory-state association that will allow to access all referees in a room in closer-to-constant time.
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
2 participants
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.
Should close #541.
The scenario here is as follows:
The reason for this bug is an oversight. The room disband operation relied on iterating over
room.Usersto clean up all users' state. This was safe to do prior to #466, because a referee drop-out would automatically incur a full leave including privilege revocation, but is not safe to do any more.To address this, change to a somewhat naive loop over all referees in the hub that ensures every single remaining one is dissociated from the closed room. I am not going to defend how it looks too hard, though extenuating circumstances here include:
If despite the above the fix is still considered too offensive to live, I will attempt to conjure some sort of inverse memory-state association that will allow to access all referees in a room in closer-to-constant time.