Net10 upgrade and threading fixes

.github/workflows/dotnet-core.yml

@@ -1,4 +1,4 @@
-name: .NET Core
+name: .NET
 
 on:
   push:
@@ -15,13 +15,13 @@ jobs:
       working-directory: ./src/Atlasd
 
     steps:
-    - uses: actions/checkout@v2
+    - uses: actions/checkout@v4
     - name: Checkout submodules
       run: git submodule update --init --recursive
-    - name: Setup .NET Core
-      uses: actions/setup-dotnet@v1
+    - name: Setup .NET
+      uses: actions/setup-dotnet@v4
       with:
-        dotnet-version: 3.1.301
+        dotnet-version: '10.0.x'
     - name: Install dependencies
       run: dotnet restore
       working-directory: ${{env.working-directory}}

src/Atlasd/Atlasd.csproj

@@ -2,7 +2,7 @@
 
   <PropertyGroup>
     <OutputType>Exe</OutputType>
-    <TargetFramework>netcoreapp3.1</TargetFramework>
+    <TargetFramework>net10.0</TargetFramework>
     <Description>Atlas is cross-platform software that emulates Classic Battle.net in a compatible model for Diablo, StarCraft, and WarCraft.</Description>
     <Copyright>© 2020-2021 Carl Bennett &lt;carl@carlbennett.me&gt;</Copyright>
     <Company>BNETDocs</Company>

src/Atlasd/Battlenet/BinaryReader.cs

@@ -1,4 +1,5 @@
-using System.IO;
+using Atlasd.Battlenet.Exceptions;
+using System.IO;
 using System.Text;
 
 namespace Atlasd.Battlenet
@@ -17,7 +18,7 @@ public long GetNextNull()
             {
                 long lastPosition = BaseStream.Position;
 
-                while (BaseStream.CanRead)
+                while (BaseStream.Position < BaseStream.Length)
                 {
                     if (ReadByte() == 0)
                     {
@@ -27,6 +28,7 @@ public long GetNextNull()
                     }
                 }
 
+                BaseStream.Position = lastPosition;
                 return -1;
             }
         }
@@ -35,7 +37,13 @@ public byte[] ReadByteString()
         {
             lock (_lock)
             {
-                var size = GetNextNull() - BaseStream.Position;
+                var nullPos = GetNextNull();
+                if (nullPos < 0)
+                {
+                    throw new GameProtocolViolationException(null,
+                        $"Truncated string field at stream position {BaseStream.Position}: missing null terminator");
+                }
+                var size = nullPos - BaseStream.Position;
                 return ReadBytes((int)size)[..^1];
             }
         }
@@ -46,7 +54,7 @@ public override string ReadString()
             {
                 string str = "";
                 char chr;
-                while ((int)(chr = ReadChar()) != 0)
+                while (BaseStream.Position < BaseStream.Length && (int)(chr = ReadChar()) != 0)
                     str += chr;
                 return str;
             }

src/Atlasd/Battlenet/Channel.cs

@@ -285,7 +285,10 @@ public void Close()
 
         public void Designate(GameState designator, GameState heir)
         {
-            DesignatedHeirs[designator] = heir;
+            lock (DesignatedHeirs)
+            {
+                DesignatedHeirs[designator] = heir;
+            }
         }
 
         public bool DisbandInto(Channel destination)

src/Atlasd/Battlenet/Common.cs

@@ -43,6 +43,7 @@ public ShutdownEvent(string adminMessage, bool cancelled, DateTime eventDate, Ti
         public static ConcurrentDictionary<byte[], Clan> ActiveClans;
         public static ConcurrentDictionary<Socket, ClientState> ActiveClientStates;
         public static List<GameAd> ActiveGameAds;
+        public static readonly object ActiveGameAdsLock = new object();
         public static ConcurrentDictionary<string, GameState> ActiveGameStates;
         public static IPAddress DefaultAddress { get; private set; }
         public static int DefaultPort { get; private set; }

src/Atlasd/Battlenet/Friend.cs

@@ -60,9 +60,20 @@ public void Sync(GameState source)
                 return;
             }
 
-            lock (source)
+            // Enforce consistent lock ordering to prevent ABBA deadlock.
+            // Always lock the object with the lower RuntimeHelpers hash first.
+            var first = source;
+            var second = target;
+            if (System.Runtime.CompilerServices.RuntimeHelpers.GetHashCode(source) >
+                System.Runtime.CompilerServices.RuntimeHelpers.GetHashCode(target))
             {
-                lock (target)
+                first = target;
+                second = source;
+            }
+
+            lock (first)
+            {
+                lock (second)
                 {
                     var admin = source.HasAdmin();
                     var mutual = false;

src/Atlasd/Battlenet/Protocols/Game/GameState.cs

@@ -19,7 +19,7 @@ public enum LogonTypes : UInt32
             NLS = 2,
         };
 
-        private bool IsDisposing = false;
+        private volatile bool IsDisposing = false;
 
         public ClientState Client { get; protected set; }
 
@@ -125,38 +125,41 @@ public static bool CanStatstringUpdate(Product.ProductCode code)
 
         public void Close()
         {
-            // Remove this GameState from ActiveChannel
-            if (ActiveChannel != null)
+            lock (this)
             {
-                ActiveChannel.RemoveUser(this); // will change this.ActiveChannel to null.
-            }
+                // Remove this GameState from ActiveChannel
+                if (ActiveChannel != null)
+                {
+                    ActiveChannel.RemoveUser(this); // will change this.ActiveChannel to null.
+                }
 
-            // Notify clan members
-            if (ActiveClan != null)
-            {
-                ActiveClan.WriteStatusChange(this, false); // offline
-            }
+                // Notify clan members
+                if (ActiveClan != null)
+                {
+                    ActiveClan.WriteStatusChange(this, false); // offline
+                }
 
-            // Update keys of ActiveAccount
-            if (ActiveAccount != null)
-            {
-                ActiveAccount.Set(Account.LastLogoffKey, DateTime.Now);
+                // Update keys of ActiveAccount
+                if (ActiveAccount != null)
+                {
+                    ActiveAccount.Set(Account.LastLogoffKey, DateTime.Now);
 
-                var timeLogged = (UInt32)ActiveAccount.Get(Account.TimeLoggedKey);
-                var diff = DateTime.Now - ConnectedTimestamp;
-                timeLogged += (UInt32)Math.Round(diff.TotalSeconds);
-                ActiveAccount.Set(Account.TimeLoggedKey, timeLogged);
-            }
+                    var timeLogged = (UInt32)ActiveAccount.Get(Account.TimeLoggedKey);
+                    var diff = DateTime.Now - ConnectedTimestamp;
+                    timeLogged += (UInt32)Math.Round(diff.TotalSeconds);
+                    ActiveAccount.Set(Account.TimeLoggedKey, timeLogged);
+                }
 
-            // Remove this OnlineName from ActiveAccounts and ActiveGameStates
-            if (!string.IsNullOrEmpty(OnlineName))
-            {
-                Battlenet.Common.ActiveAccounts.TryRemove(OnlineName, out _);
-                Battlenet.Common.ActiveGameStates.TryRemove(OnlineName, out _);
-            }
+                // Remove this OnlineName from ActiveAccounts and ActiveGameStates
+                if (!string.IsNullOrEmpty(OnlineName))
+                {
+                    Battlenet.Common.ActiveAccounts.TryRemove(OnlineName, out _);
+                    Battlenet.Common.ActiveGameStates.TryRemove(OnlineName, out _);
+                }
 
-            // Remove this GameAd
-            if (GameAd != null && GameAd.RemoveClient(this)) GameAd = null;
+                // Remove this GameAd
+                if (GameAd != null && GameAd.RemoveClient(this)) GameAd = null;
+            }
         }
 
         public void Dispose() /* part of IDisposable */

src/Atlasd/Battlenet/Protocols/Game/Messages/SID_ENTERCHAT.cs

@@ -102,7 +102,7 @@ public override bool Invoke(MessageContext context)
                             {
                                 var gameAd = gameState.GameAd;
                                 if (gameAd.RemoveClient(gameState)) gameState.GameAd = null;
-                                if (gameAd.Clients.Count == 0) lock (Battlenet.Common.ActiveGameAds) Battlenet.Common.ActiveGameAds.Remove(gameAd);
+                                if (gameAd.Clients.Count == 0) lock (Battlenet.Common.ActiveGameAdsLock) Battlenet.Common.ActiveGameAds.Remove(gameAd);
                             }
 
                             if (gameState.ActiveChannel == null)

src/Atlasd/Battlenet/Protocols/Game/Messages/SID_GETADVLISTEX.cs

@@ -58,7 +58,7 @@ public override bool Invoke(MessageContext context)
 
                         var gameAds = new List<GameAd>();
 
-                        lock (Battlenet.Common.ActiveGameAds)
+                        lock (Battlenet.Common.ActiveGameAdsLock)
                         {
                             IList<GameAd> toDelete = new List<GameAd>();
                             foreach (var gameAd in Battlenet.Common.ActiveGameAds)

src/Atlasd/Battlenet/Protocols/Game/Messages/SID_NOTIFYJOIN.cs

@@ -52,7 +52,7 @@ public override bool Invoke(MessageContext context)
             if (gameState.ActiveChannel != null)
                 gameState.ActiveChannel.RemoveUser(gameState);
 
-            lock (Battlenet.Common.ActiveGameAds)
+            lock (Battlenet.Common.ActiveGameAdsLock)
             {
                 foreach (var gameAd in Battlenet.Common.ActiveGameAds)
                 {

src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STARTADVEX.cs

@@ -69,7 +69,7 @@ public override bool Invoke(MessageContext context)
                         Statuses status = Statuses.Failed;
                         GameAd gameAd = null;
 
-                        lock (Battlenet.Common.ActiveGameAds)
+                        lock (Battlenet.Common.ActiveGameAdsLock)
                         {
                             foreach (GameAd _ad in Battlenet.Common.ActiveGameAds)
                             {

src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STARTADVEX2.cs

@@ -69,7 +69,7 @@ public override bool Invoke(MessageContext context)
                         Statuses status = Statuses.Failed;
                         GameAd gameAd = null;
 
-                        lock (Battlenet.Common.ActiveGameAds)
+                        lock (Battlenet.Common.ActiveGameAdsLock)
                         {
                             foreach (GameAd _ad in Battlenet.Common.ActiveGameAds)
                             {

src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STARTADVEX3.cs

@@ -73,7 +73,7 @@ public override bool Invoke(MessageContext context)
                         Statuses status = Statuses.Error;
                         GameAd gameAd = null;
 
-                        lock (Battlenet.Common.ActiveGameAds)
+                        lock (Battlenet.Common.ActiveGameAdsLock)
                         {
                             foreach (GameAd _ad in Battlenet.Common.ActiveGameAds)
                             {

src/Atlasd/Battlenet/Protocols/Game/Messages/SID_STOPADV.cs

@@ -42,7 +42,7 @@ public override bool Invoke(MessageContext context)
             if (!gameAdOwner)
                 Logging.WriteLine(Logging.LogLevel.Info, Logging.LogType.Client_Game, context.Client.RemoteEndPoint, $"{MessageName(Id)} was received but they are not the owner of the game advertisement");
             else
-                Battlenet.Common.ActiveGameAds.Remove(gs.GameAd);
+                lock (Battlenet.Common.ActiveGameAdsLock) Battlenet.Common.ActiveGameAds.Remove(gs.GameAd);
 
             return true;
         }

src/Atlasd/Battlenet/Protocols/HTTP/HttpListener.cs

@@ -64,7 +64,6 @@ public void Start() /* part of IListener */
             {
                 ExclusiveAddressUse = true,
                 NoDelay = Daemon.Common.TcpNoDelay,
-                UseOnlyOverlappedIO = true,
             };
             Socket.Bind(LocalEndPoint);
             Socket.Listen(-1);

src/Atlasd/Battlenet/ServerSocket.cs

@@ -82,7 +82,6 @@ public void SetLocalEndPoint(IPEndPoint localEndPoint)
             {
                 ExclusiveAddressUse = true,
                 NoDelay = Daemon.Common.TcpNoDelay,
-                UseOnlyOverlappedIO = true,
             };
         }