Fix issues with concurrent connection filtering

[bdach]

The hope here is that this closes ppy/osu#25635 and fixes similar issues that have been visible on sentry for the past day or so, but I can't provide any strong guarantee that it does, as testing this in practice is difficult.

The goal here is to fix two shortcomings:

• Because each hub is a separate endpoint / websocket connection, they do not have to be synchronised wrt connection state at all. Therefore, it is possible for the user to disconnect from only some of the hubs (and I have observed this in practice). The previous cleanup logic implicitly assumed this to be impossible, and as such nuked the user's entire state. To counteract this, each disconnect event only performs cleanup in the context of the hub where the disconnect happened, and the entity is removed from the store if and only if it has no connections registered any more.

  This is covered by TestHubDisconnectsTrackedSeparately() in the tests. It also can be tested in practice by e.g. launching a game instance in debug and pausing its execution until signalr-side keepalive detects some hubs as dead, and then resuming the game instance. Doing so should result in a sequence similar to

  2023-12-01 10:33:41 [verbose]: [user:13] [connection:70q_Und49nat8Ky1vQYHZw] [hub:MultiplayerHub] connection from first client instance
  2023-12-01 10:33:41 [verbose]: [user:13] [connection:fZ5XUJpztt3HkTGeRpAPaQ] [hub:MetadataHub] subsequent connection from same client instance, registering
  2023-12-01 10:33:41 [verbose]: [user:13] [connection:pXga_043AKYWsifLMLXz0Q] [hub:SpectatorHub] subsequent connection from same client instance, registering
  2023-12-01 10:34:59 [verbose]: [user:13] [connection:70q_Und49nat8Ky1vQYHZw] [hub:MultiplayerHub] disconnected from hub
  2023-12-01 10:34:59 [verbose]: [user:13] [connection:pXga_043AKYWsifLMLXz0Q] [hub:SpectatorHub] disconnected from hub
  2023-12-01 10:35:01 [verbose]: [user:13] [connection:UXsCivQPhyGUoUWCsOYjyQ] [hub:MultiplayerHub] subsequent connection from same client instance, registering
  2023-12-01 10:35:06 [verbose]: [user:13] [connection:x4_gO7ftW8cSIQEMl_IbVQ] [hub:SpectatorHub] subsequent connection from same client instance, registering
  

  with this pull.

• Another scenario pointed out by @peppy - theoretical at this point, although plausible - is that if the following sequence of events happens (using server-side message ordering) for a single client instance:

  • Connection A with hub X established
  • Connection B with hub X established due to client-side reconnection
  • Connection A with hub X closed cleanly

  then the last disconnection would not only nuke the entire connection state as mentioned in the previous point, it shouldn't really be doing anything at all, since it's a stale event concerning a stale connection and therefore it should not result in anything changing. Thus, a connection disconnection event can only unregister a connection now if the connection IDs match.

  This is covered in the added unit tests, in TestStaleDisconnectIsANoOp().

[peppy]

👍