[Bug]: Gracefully handle relogs and timeouts (possible mishandling of sessions/connections?)

[jonbarrow]

Checked Existing

• I have checked the repository for duplicate issues.

What happened?

Currently when a user logs in and out there is a chance for the server to treat the client as still connected. This can be seen in Imora's network dumps, where the console relogs quickly and packets from the server are still being sent.

What did you expect to happen?

The server should probably have a better timeout mechanism for this? I'm unsure how to acomplish this though. We did not see these kinds of issues in Imora's dumps when he used Nintendo Network, so clearly they're doing something better.

#48 may also help here? Relogs won't matter if we just reset the client state whenever a new SYN is sent.

However we may also need to change how we handle sessions? Looking back over the code, it seems like this is half-baked. PRUDPConnection represents a single PRUDP connection, and has a reference to SocketConnection. SocketConnection represents a physical socket connection (UDP or WebSocket), and has a reference to all the PRUDP connections attached to it. However I can't seem to find a place where we actually set the PRUDPConnection on the SocketConnection?

SocketConnection.Connections comment says:

Open PRUDP connections separated by rdv::Stream ID, also called "port number"

However when this is used in PRUDPConnection.cleanup the connections session ID is used?

pc.Socket.Connections.Delete(pc.SessionID)

This seems very wrong, and is at least partially to blame for some issues. We seem to be mishandling sessions/connections here.

Steps to reproduce? (OPTIONAL)

No response

Any other details to share? (OPTIONAL)

No response

[DaniElectra]

We have previously removed the timeout DISCONNECT packets, but I think these could help when debugging this type of issues, so that we can catch relogs on network dumps easily and prevent confusion with other network issues from the user

The SocketConnection being unused also seems interesting

[jonbarrow]

We have previously removed the timeout DISCONNECT packets

I agree with reimplementing these. But I don't think they will fix all the issues. Pablo has mentioned that Mario Kart 7 does not handle these kinds of packets from the server the same way other games do, so it may be somewhat unreliable to lean on

Also it seems like timeouts just aren't happening correctly in general? Sending the DISCONNECT packet works in cases where

1. The client is connected
2. The server knows to time the client out

But according to dumps from Imora, the server seems to still be trying to send the client DATA packets long after the server should have timed out the connection? Which makes me think it wouldn't be sending the DISCONNECT packet at all in those cases

[ashquarky]

Note that even after #76 there is still some source of connections that aren't removed from the Connections map, though these new ones seem genuinely harmless outside the leaked memory and slower map lookups

[ashquarky]

(Minecraft: Wii U edition has 446 entries in its Connections map after 3 days of uptime)

[DaniElectra]

I believe the missing stalled connections would be from clients from which a CONNECT packet is never received on the server, since the timers are never initialized in those cases and thus no one cleans up those connections. The original intention was to avoid sending ping packets to clients that were still in the connecting phase, but this also causes some stalled connections too