From 216029dc27ca389f31eb23b9783490f92fa6d4fb Mon Sep 17 00:00:00 2001
From: penev92 <penev92@gmail.com>
Date: Sat, 4 Jun 2022 10:30:21 +0300
Subject: [PATCH] Fix a crash with BlockingCollection in Connection

The BlockingCollection would have `IsAddingCompleted` to true, but `IsComplete` to false, slipping through the cracks and causing an InvalidOperationException ("The collection has been marked as complete with regards to additions.") when trying to add to it.
We now add a check on `(Try)SendData` to only try to add if we can. The collection is still viable for reading until empty/`IsComplete`.
---
 OpenRA.Game/Server/Connection.cs | 22 ++++++++++++++++------
 OpenRA.Game/Server/Server.cs     | 10 +++-------
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/OpenRA.Game/Server/Connection.cs b/OpenRA.Game/Server/Connection.cs
index 7f1714bdd5..4bb1d76aa5 100644
--- a/OpenRA.Game/Server/Connection.cs
+++ b/OpenRA.Game/Server/Connection.cs
@@ -154,10 +154,8 @@ namespace OpenRA.Server
 
 					// Regularly check player ping
 					if (lastPingSent.ElapsedMilliseconds > 1000)
-					{
-						sendQueue.Add(CreatePingFrame());
-						lastPingSent.Restart();
-					}
+						if (TrySendData(CreatePingFrame()))
+							lastPingSent.Restart();
 
 					// Send all data immediately, we will block again on read
 					while (sendQueue.TryTake(out var data, 0))
@@ -195,9 +193,21 @@ namespace OpenRA.Server
 			}
 		}
 
-		public void SendData(byte[] data)
+		public bool TrySendData(byte[] data)
 		{
-			sendQueue.Add(data);
+			if (sendQueue.IsAddingCompleted)
+				return false;
+
+			try
+			{
+				sendQueue.Add(data);
+				return true;
+			}
+			catch (InvalidOperationException)
+			{
+				// Occurs if the collection is marked completed for adding by another thread.
+				return false;
+			}
 		}
 
 		public void Dispose()
diff --git a/OpenRA.Game/Server/Server.cs b/OpenRA.Game/Server/Server.cs
index 24550c7dc7..6fd59e1a41 100644
--- a/OpenRA.Game/Server/Server.cs
+++ b/OpenRA.Game/Server/Server.cs
@@ -434,7 +434,7 @@ namespace OpenRA.Server
 				var ms = new MemoryStream(8);
 				ms.WriteArray(BitConverter.GetBytes(ProtocolVersion.Handshake));
 				ms.WriteArray(BitConverter.GetBytes(newConn.PlayerIndex));
-				newConn.SendData(ms.ToArray());
+				newConn.TrySendData(ms.ToArray());
 
 				// Dispatch a handshake order
 				var request = new HandshakeRequest
@@ -737,14 +737,10 @@ namespace OpenRA.Server
 
 		void DispatchFrameToClient(Connection c, int client, byte[] frameData)
 		{
-			try
-			{
-				c.SendData(frameData);
-			}
-			catch (Exception e)
+			if (!c.TrySendData(frameData))
 			{
 				DropClient(c);
-				Log.Write("server", $"Dropping client {client.ToString(CultureInfo.InvariantCulture)} because dispatching orders failed: {e}");
+				Log.Write("server", $"Dropping client {client.ToString(CultureInfo.InvariantCulture)} because dispatching orders failed!");
 			}
 		}