From 69cef89953ea6eb58956388408c4afb1591e48a0 Mon Sep 17 00:00:00 2001
From: "Robin C. Ladiges" <rcl.git@blackpinguin.de>
Date: Sat, 3 Sep 2022 23:04:01 +0200
Subject: [PATCH] fix: on reconnect do not disconnect the new client

Currently when a client connects that is already there,
the old socket is closed, and the code tries to reuse the existing client object by exchanging its socket.

Reusing the same client object and just changing its socket does cause issues though with copies of the client in other threads.
In the situations that I could reproduce, it always disconnected both sockets, the old one and then the new one.

Instead I make a copy of the client object, use the new socket, remove the old object and add the new object to the collection.

(cherry picked from commit 9e6c312c8e2f4967cf08a6d7c46f6c7779f847f3)
---
 Server/Client.cs | 13 ++++++++++++-
 Server/Server.cs | 32 ++++++++++----------------------
 2 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/Server/Client.cs b/Server/Client.cs
index 7ba967f..5f3336f 100644
--- a/Server/Client.cs
+++ b/Server/Client.cs
@@ -28,6 +28,17 @@ public class Client : IDisposable {
         Logger = new Logger("Unknown User");
     }
 
+    // copy Client to use existing data for a new reconnected connection with a new socket
+    public Client(Client other, Socket socket) {
+        Metadata       = other.Metadata;
+        Connected      = other.Connected;
+        CurrentCostume = other.CurrentCostume;
+        Id             = other.Id;
+        Socket         = socket;
+        Server         = other.Server;
+        Logger         = other.Logger;
+    }
+
     public void Dispose() {
         if (Socket?.Connected is true)
             Socket.Disconnect(false);
@@ -83,4 +94,4 @@ public class Client : IDisposable {
     public override int GetHashCode() {
         return Id.GetHashCode(); //relies upon same info as == operator.
     }
-}
\ No newline at end of file
+}
diff --git a/Server/Server.cs b/Server/Server.cs
index 4f973ac..faef82c 100644
--- a/Server/Server.cs
+++ b/Server/Server.cs
@@ -180,32 +180,21 @@ public class Server {
                             goto disconnect;
                         }
 
-                        bool firstConn = false;
+                        bool firstConn = true;
                         switch (connect.ConnectionType) {
-                            case ConnectPacket.ConnectionTypes.FirstConnection: {
-                                firstConn = true;
-                                if (FindExistingClient(header.Id) is { } newClient) {
-                                    if (newClient.Connected) {
-                                        newClient.Logger.Info($"Disconnecting already connected client {newClient.Socket?.RemoteEndPoint} for {client.Socket?.RemoteEndPoint}");
-                                        newClient.Dispose();
-                                    }
-                                    newClient.Socket = client.Socket;
-                                    client = newClient;
-                                }
-
-                                break;
-                            }
+                            case ConnectPacket.ConnectionTypes.FirstConnection:
                             case ConnectPacket.ConnectionTypes.Reconnecting: {
                                 client.Id = header.Id;
-                                if (FindExistingClient(header.Id) is { } newClient) {
-                                    if (newClient.Connected) {
-                                        newClient.Logger.Info($"Disconnecting already connected client {newClient.Socket?.RemoteEndPoint} for {client.Socket?.RemoteEndPoint}");
-                                        newClient.Dispose();
+                                if (FindExistingClient(header.Id) is { } oldClient) {
+                                    firstConn = false;
+                                    client = new Client(oldClient, socket);
+                                    Clients.Remove(oldClient);
+                                    Clients.Add(client);
+                                    if (oldClient.Connected) {
+                                        oldClient.Logger.Info($"Disconnecting already connected client {oldClient.Socket?.RemoteEndPoint} for {client.Socket?.RemoteEndPoint}");
+                                        oldClient.Dispose();
                                     }
-                                    newClient.Socket = client.Socket;
-                                    client = newClient;
                                 } else {
-                                    firstConn = true;
                                     connect.ConnectionType = ConnectPacket.ConnectionTypes.FirstConnection;
                                 }
 
@@ -222,7 +211,6 @@ public class Server {
                             List<Client> toDisconnect = Clients.FindAll(c => c.Id == header.Id && c.Connected && c.Socket != null);
                             Clients.RemoveAll(c => c.Id == header.Id);
 
-                            client.Id = header.Id;
                             Clients.Add(client);
 
                             Parallel.ForEachAsync(toDisconnect, (c, token) => c.Socket!.DisconnectAsync(false, token));