check nulls remove redundant catches.

LightlessSync/PlayerData/Factories/PlayerDataFactory.cs

@@ -11,6 +11,8 @@ using LightlessSync.Services.Mediator;
 using Microsoft.Extensions.Logging;
 using System.Collections.Concurrent;
 using System.Diagnostics;
+using System.Runtime.ExceptionServices;
+using System.Runtime.InteropServices;
 
 namespace LightlessSync.PlayerData.Factories;
 
@@ -123,22 +125,38 @@ public class PlayerDataFactory
     {
         if (playerPointer == IntPtr.Zero)
             return true;
+
+        if (!IsPointerValid(playerPointer))
+            return true;
+
+        var character = (Character*)playerPointer;
+        if (character == null)
+            return true;
+
+        var gameObject = &character->GameObject;
+        if (gameObject == null)
+            return true;
+
+        if (!IsPointerValid((IntPtr)gameObject))
+            return true;
+
+        return gameObject->DrawObject == null;
+    }
+
+    private static bool IsPointerValid(IntPtr ptr)
+    {
+        if (ptr == IntPtr.Zero)
+            return false;
+
         try
         {
-            var character = (Character*)playerPointer;
+            _ = Marshal.ReadByte(ptr);
-            if (character == null)
-                return true;
-
-            var gameObject = &character->GameObject;
-            if (gameObject == null)
-                return true;
-
-            return gameObject->DrawObject == null;
-        }
-        catch (AccessViolationException)
-        {
             return true;
         }
+        catch
+        {
+            return false;
+        }
     }
 
     private static bool IsCacheFresh(CacheEntry entry)

LightlessSync/Services/DalamudUtilService.cs

@@ -666,7 +666,7 @@ public class DalamudUtilService : IHostedService, IMediatorSubscriber
 
         var location = new LocationInfo();
         location.ServerId = _playerState.CurrentWorld.RowId;
-        location.InstanceId = UIState.Instance()->PublicInstance.InstanceId; 
+        location.InstanceId = UIState.Instance()->PublicInstance.InstanceId;
         location.TerritoryId = _clientState.TerritoryType;
         location.MapId = _clientState.MapId;
         if (houseMan != null)
@@ -699,13 +699,13 @@ public class DalamudUtilService : IHostedService, IMediatorSubscriber
         }
         return location;
     }
-    
+
     public string LocationToString(LocationInfo location)
     {
         if (location.ServerId is 0 || location.TerritoryId is 0) return String.Empty;
         var str = WorldData.Value[(ushort)location.ServerId];
 
-        if (ContentFinderData.Value.TryGetValue(location.TerritoryId , out var dutyName))
+        if (ContentFinderData.Value.TryGetValue(location.TerritoryId, out var dutyName))
         {
             str += $" - [In Duty]{dutyName}";
         }
@@ -856,7 +856,7 @@ public class DalamudUtilService : IHostedService, IMediatorSubscriber
 
         var token = ct ?? CancellationToken.None;
 
-        const int tick = 250; 
+        const int tick = 250;
         const int initialSettle = 50;
 
         var sw = Stopwatch.StartNew();
@@ -881,7 +881,7 @@ public class DalamudUtilService : IHostedService, IMediatorSubscriber
         {
             // ignore
         }
-        catch (AccessViolationException ex)
+        catch (Exception ex)
         {
             logger.LogWarning(ex, "Error accessing {handler}, object does not exist anymore?", handler);
         }
@@ -922,11 +922,11 @@ public class DalamudUtilService : IHostedService, IMediatorSubscriber
     public string? GetWorldNameFromPlayerAddress(nint address)
     {
         if (address == nint.Zero) return null;
-        
+
         EnsureIsOnFramework();
         var playerCharacter = _objectTable.OfType<IPlayerCharacter>().FirstOrDefault(p => p.Address == address);
         if (playerCharacter == null) return null;
-        
+
         var worldId = (ushort)playerCharacter.HomeWorld.RowId;
         return WorldData.Value.TryGetValue(worldId, out var worldName) ? worldName : null;
     }
@@ -953,105 +953,108 @@ public class DalamudUtilService : IHostedService, IMediatorSubscriber
         });
     }
 
+    [DllImport("kernel32.dll", SetLastError = true)]
+    private static extern bool IsBadReadPtr(IntPtr ptr, UIntPtr size);
+
+    private static bool IsValidPointer(nint ptr, int size = 8)
+    {
+        if (ptr == nint.Zero)
+            return false;
+
+        try
+        {
+            if (!Util.IsWine())
+            {
+                return !IsBadReadPtr(ptr, (UIntPtr)size);
+            }
+            return ptr != nint.Zero && (ptr % IntPtr.Size) == 0;
+        }
+        catch
+        {
+            return false;
+        }
+    }
+
     private unsafe void CheckCharacterForDrawing(nint address, string characterName)
     {
         if (address == nint.Zero)
             return;
 
-        try
+        if (!IsValidPointer(address))
         {
-            var gameObj = (GameObject*)address;
+            _logger.LogDebug("Invalid pointer for character {name} at {addr}", characterName, address.ToString("X"));
+            return;
+        }
 
-            if (gameObj == null)
+        var gameObj = (GameObject*)address;
-                return;
 
-            if (!_objectTable.Any(o => o?.Address == address))
+        if (gameObj == null)
+            return;
+
+        if (!_objectTable.Any(o => o?.Address == address))
+        {
+            _logger.LogDebug("Character {name} at {addr} no longer in object table", characterName, address.ToString("X"));
+            return;
+        }
+
+        if (gameObj->ObjectKind == 0)
+            return;
+
+        var drawObj = gameObj->DrawObject;
+        bool isDrawing = false;
+        bool isDrawingChanged = false;
+
+        if ((nint)drawObj != IntPtr.Zero && IsValidPointer((nint)drawObj))
+        {
+            isDrawing = gameObj->RenderFlags == (VisibilityFlags)0b100000000000;
+
+            if (!isDrawing)
             {
-                _logger.LogDebug("Character {name} at {addr} no longer in object table", characterName, address.ToString("X"));
+                var charBase = (CharacterBase*)drawObj;
-                return;
+                if (charBase != null && IsValidPointer((nint)charBase))
-            }
-
-            if (gameObj->ObjectKind == 0)
-                return;
-
-            var drawObj = gameObj->DrawObject;
-            bool isDrawing = false;
-            bool isDrawingChanged = false;
-
-            if ((nint)drawObj != IntPtr.Zero)
-            {
-                try
                 {
-                    isDrawing = gameObj->RenderFlags == (VisibilityFlags)0b100000000000;
+                    isDrawing = charBase->HasModelInSlotLoaded != 0;
-                }
+                    if (!isDrawing)
-                catch (AccessViolationException)
-                {
-                    return;
-                }
-
-                if (!isDrawing)
-                {
-                    try
                     {
-                        var charBase = (CharacterBase*)drawObj;
+                        isDrawing = charBase->HasModelFilesInSlotLoaded != 0;
-                        if (charBase != null)
+                        if (isDrawing && !string.Equals(_lastGlobalBlockPlayer, characterName, StringComparison.Ordinal)
+                            && !string.Equals(_lastGlobalBlockReason, "HasModelFilesInSlotLoaded", StringComparison.Ordinal))
                         {
-                            isDrawing = charBase->HasModelInSlotLoaded != 0;
+                            _lastGlobalBlockPlayer = characterName;
-                            if (!isDrawing)
+                            _lastGlobalBlockReason = "HasModelFilesInSlotLoaded";
-                            {
+                            isDrawingChanged = true;
-                                isDrawing = charBase->HasModelFilesInSlotLoaded != 0;
-                                if (isDrawing && !string.Equals(_lastGlobalBlockPlayer, characterName, StringComparison.Ordinal)
-                                    && !string.Equals(_lastGlobalBlockReason, "HasModelFilesInSlotLoaded", StringComparison.Ordinal))
-                                {
-                                    _lastGlobalBlockPlayer = characterName;
-                                    _lastGlobalBlockReason = "HasModelFilesInSlotLoaded";
-                                    isDrawingChanged = true;
-                                }
-                            }
-                            else
-                            {
-                                if (!string.Equals(_lastGlobalBlockPlayer, characterName, StringComparison.Ordinal)
-                                    && !string.Equals(_lastGlobalBlockReason, "HasModelInSlotLoaded", StringComparison.Ordinal))
-                                {
-                                    _lastGlobalBlockPlayer = characterName;
-                                    _lastGlobalBlockReason = "HasModelInSlotLoaded";
-                                    isDrawingChanged = true;
-                                }
-                            }
                         }
                     }
-                    catch (AccessViolationException)
+                    else
                     {
-                        return;
+                        if (!string.Equals(_lastGlobalBlockPlayer, characterName, StringComparison.Ordinal)
-                    }
+                            && !string.Equals(_lastGlobalBlockReason, "HasModelInSlotLoaded", StringComparison.Ordinal))
-                }
+                        {
-                else
+                            _lastGlobalBlockPlayer = characterName;
-                {
+                            _lastGlobalBlockReason = "HasModelInSlotLoaded";
-                    if (!string.Equals(_lastGlobalBlockPlayer, characterName, StringComparison.Ordinal)
+                            isDrawingChanged = true;
-                        && !string.Equals(_lastGlobalBlockReason, "RenderFlags", StringComparison.Ordinal))
+                        }
-                    {
-                        _lastGlobalBlockPlayer = characterName;
-                        _lastGlobalBlockReason = "RenderFlags";
-                        isDrawingChanged = true;
                     }
                 }
             }
-
+            else
-            if (isDrawingChanged)
             {
-                _logger.LogTrace("Global draw block: START => {name} ({reason})", characterName, _lastGlobalBlockReason);
+                if (!string.Equals(_lastGlobalBlockPlayer, characterName, StringComparison.Ordinal)
+                    && !string.Equals(_lastGlobalBlockReason, "RenderFlags", StringComparison.Ordinal))
+                {
+                    _lastGlobalBlockPlayer = characterName;
+                    _lastGlobalBlockReason = "RenderFlags";
+                    isDrawingChanged = true;
+                }
             }
+        }
 
-            IsAnythingDrawing |= isDrawing;
+        if (isDrawingChanged)
-        }
-        catch (AccessViolationException ex)
         {
-            _logger.LogDebug(ex, "Memory access violation checking character {name} at {addr}", characterName, address.ToString("X"));
+            _logger.LogTrace("Global draw block: START => {name} ({reason})", characterName, _lastGlobalBlockReason);
-        }
-        catch (Exception ex)
-        {
-            _logger.LogWarning(ex, "Unexpected error checking character {name} at {addr}", characterName, address.ToString("X"));
         }
+
+        IsAnythingDrawing |= isDrawing;
     }
 
     private void FrameworkOnUpdate(IFramework framework)
@@ -1061,6 +1064,11 @@ public class DalamudUtilService : IHostedService, IMediatorSubscriber
 
     private unsafe void FrameworkOnUpdateInternal()
     {
+        if (!_clientState.IsLoggedIn || _objectTable.LocalPlayer == null)
+        {
+            return;
+        }
+
         if ((_objectTable.LocalPlayer?.IsDead ?? false) && _condition[ConditionFlag.BoundByDuty])
         {
             return;
@@ -1084,70 +1092,38 @@ public class DalamudUtilService : IHostedService, IMediatorSubscriber
 
                     for (var i = 0; i < descriptorCount; i++)
                     {
-                        try
+                        if (i >= playerDescriptors.Count)
+                            break;
+
+                        var actor = playerDescriptors[i];
+
+                        var playerAddress = actor.Address;
+                        if (playerAddress == nint.Zero || !IsValidPointer(playerAddress))
+                            continue;
+
+                        if (actor.ObjectIndex >= 200)
+                            continue;
+
+                        if (_blockedCharacterHandler.IsCharacterBlocked(playerAddress, actor.ObjectIndex, out bool firstTime) && firstTime)
                         {
-                            if (i >= playerDescriptors.Count)
+                            _logger.LogTrace("Skipping character {addr}, blocked/muted", playerAddress.ToString("X"));
+                            continue;
+                        }
+
+                        if (!IsAnythingDrawing)
+                        {
+                            if (!_objectTable.Any(o => o?.Address == playerAddress))
+                            {
+                                continue;
+                            }
+
+                            CheckCharacterForDrawing(playerAddress, actor.Name);
+
+                            if (IsAnythingDrawing)
                                 break;
-
-                            var actor = playerDescriptors[i];
-
-                            var playerAddress = actor.Address;
-                            if (playerAddress == nint.Zero)
-                                continue;
-
-                            if (actor.ObjectIndex >= 200)
-                                continue;
-
-                            if (_blockedCharacterHandler.IsCharacterBlocked(playerAddress, actor.ObjectIndex, out bool firstTime) && firstTime)
-                            {
-                                _logger.LogTrace("Skipping character {addr}, blocked/muted", playerAddress.ToString("X"));
-                                continue;
-                            }
-
-                            if (!IsAnythingDrawing)
-                            {
-                                try
-                                {
-                                    var gameObj = (GameObject*)playerAddress;
-
-                                    if (gameObj == null || gameObj->ObjectKind == 0)
-                                    {
-                                        continue;
-                                    }
-
-                                    string currentName;
-                                    try
-                                    {
-                                        currentName = gameObj->NameString ?? string.Empty;
-                                    }
-                                    catch (AccessViolationException)
-                                    {
-                                        currentName = string.Empty;
-                                    }
-
-                                    var charaName = string.IsNullOrEmpty(currentName) ? actor.Name : currentName;
-
-                                    CheckCharacterForDrawing(playerAddress, charaName);
-
-                                    if (IsAnythingDrawing)
-                                        break;
-                                }
-                                catch (AccessViolationException ex)
-                                {
-                                    _logger.LogDebug(ex, "Access violation on GameObject pointer for actor {index} at {addr}", i, playerAddress.ToString("X"));
-                                }
-                            }
-                        }
-                        catch (AccessViolationException ex)
-                        {
-                            _logger.LogDebug(ex, "Access violation processing actor {index} - object likely destroyed", i);
-                        }
-                        catch (Exception ex)
-                        {
-                            _logger.LogWarning(ex, "Unexpected error processing actor {index}", i);
                         }
                     }
-             });
+                });
 
             if (!IsAnythingDrawing && !string.IsNullOrEmpty(_lastGlobalBlockPlayer))
             {
@@ -1214,7 +1190,7 @@ public class DalamudUtilService : IHostedService, IMediatorSubscriber
                 });
 
             // Cutscene
-            HandleStateTransition(() => IsInCutscene,v => IsInCutscene = v, shouldBeInCutscene, "Cutscene",
+            HandleStateTransition(() => IsInCutscene, v => IsInCutscene = v, shouldBeInCutscene, "Cutscene",
                 onEnter: () =>
                 {
                     Mediator.Publish(new CutsceneStartMessage());
@@ -1257,7 +1233,7 @@ public class DalamudUtilService : IHostedService, IMediatorSubscriber
                 Mediator.Publish(new ZoneSwitchEndMessage());
                 Mediator.Publish(new ResumeScanMessage(nameof(ConditionFlag.BetweenAreas)));
             }
-            
+
             //Map
             if (!_sentBetweenAreas)
             {
@@ -1268,7 +1244,7 @@ public class DalamudUtilService : IHostedService, IMediatorSubscriber
                     Mediator.Publish(new MapChangedMessage(mapid));
                 }
             }
-            
+
 
             var localPlayer = _objectTable.LocalPlayer;
             if (localPlayer != null)
@@ -1354,4 +1330,4 @@ public class DalamudUtilService : IHostedService, IMediatorSubscriber
             onExit();
         }
     }
-}
+}