feat: Adds an Item/Mobile memory leak detector. Fixes minor leak in doors. (#2159)

### Summary

Adds the command [TrackLeaks to enable tracking item/mobiles that have been deleted but still have dangling references. Requires adding the _TRACK_LEAKS_ define constant during build.
This commit is contained in:
Kamron Batman 2025-04-15 19:14:45 -07:00 committed by GitHub
parent 1e349f4369
commit 4aa272d429
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 256 additions and 9 deletions

View file

@ -0,0 +1,177 @@
/*************************************************************************
* ModernUO *
* Copyright 2019-2025 - ModernUO Development Team *
* Email: hi@modernuo.com *
* File: EntityFinalizationTracker.cs *
* *
* This program is free software: you can redistribute it and/or modify *
* it under the terms of the GNU General Public License as published by *
* the Free Software Foundation, either version 3 of the License, or *
* (at your option) any later version. *
* *
* You should have received a copy of the GNU General Public License *
* along with this program. If not, see <http://www.gnu.org/licenses/>. *
*************************************************************************/
using System;
using System.Collections.Generic;
using System.Runtime.CompilerServices;
using System.Threading;
using Server.Logging;
namespace Server;
public static class EntityFinalizationTracker
{
private static readonly ILogger logger = LogFactory.GetLogger(typeof(EntityFinalizationTracker));
private sealed record TrackedEntity(WeakReference<IEntity> Reference, int ReferenceHash, DateTime RemovedAt);
private static readonly Lock _sync = new();
private static bool _enabled;
private static DateTime _nextCheck = DateTime.MinValue;
#if TRACK_LEAKS
private const bool CanBeEnabled = true;
#else
private const bool CanBeEnabled = false;
#endif
public static void Configure()
{
CommandSystem.Register("gc", AccessLevel.Administrator, _ => GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced, blocking: true, compacting: true));
CommandSystem.Register("TrackLeaks", AccessLevel.Developer, TrackLeaks_OnCommand);
}
[Usage("TrackLeaks <on|off>")]
[Description("Enables or disables entity leak tracking. May impact performance and should be used only when necessary!")]
private static void TrackLeaks_OnCommand(CommandEventArgs e)
{
var from = e.Mobile;
if (!CanBeEnabled) {
from.SendMessage("Entity leak tracking is not enabled in this build. Rebuild with TRACK_LEAKS defined.");
return;
}
if (e.Arguments.Length == 0)
{
from.SendMessage("Usage: TrackLeaks <on|off>");
return;
}
var enable = Utility.ToBoolean(e.Arguments[0]);
var status = enable ? "enabled" : "disabled";
if (enable == _enabled)
{
from.SendMessage($"Entity leak tracking already {status}.");
return;
}
if (_enabled)
{
EnableLeakTracking(from);
}
else
{
DisableLeakTracking(from);
}
}
private static void EnableLeakTracking(Mobile from)
{
if (_enabled)
{
return;
}
_enabled = true;
Gen2GcCallback.Register(() =>
{
CheckLeaks();
return _enabled;
});
from.SendMessage("Entity leak tracking enabled.");
logger.Warning("Entity leak tracking enabled by {Name} ({Serial:X8})", from.Name, from.Serial);
}
private static void DisableLeakTracking(Mobile from)
{
if (!_enabled)
{
return;
}
_enabled = false;
from.SendMessage("Entity leak tracking disabled.");
logger.Warning("Entity leak tracking disabled by {Name} ({Serial:X8})", from.Name, from.Serial);
}
private static readonly List<TrackedEntity> _entities = [];
private static readonly HashSet<int> _finalizedHashes = [];
public static void TrackEntity<T>(T entity) where T : IEntity
{
lock (_sync)
{
var hash = RuntimeHelpers.GetHashCode(entity);
_entities.Add(new TrackedEntity(new WeakReference<IEntity>(entity), hash, Core.Now));
}
}
public static void NotifyFinalized(object entity)
{
lock (_sync)
{
_finalizedHashes.Add(RuntimeHelpers.GetHashCode(entity));
}
}
private static void CheckLeaks()
{
if (_entities.Count == 0)
{
return;
}
var now = Core.Now;
if (now <= _nextCheck)
{
return;
}
_nextCheck = now + TimeSpan.FromMinutes(2);
_entities.RemoveAll(entry =>
{
if (_finalizedHashes.Remove(entry.ReferenceHash))
{
return true;
}
if (!entry.Reference.TryGetTarget(out var obj) || obj is not IEntity entity)
{
return true;
}
if (ExceptionToLeakCheck(entity))
{
return false;
}
var duration = now - entry.RemovedAt;
if (duration <= TimeSpan.FromSeconds(120))
{
return false;
}
logger.Warning("[Leak Warning] {Name} ({Serial:X8}) collected but not finalized after {Duration}.", entity.GetType().Name, entity.Serial, duration);
return false;
});
}
private static bool ExceptionToLeakCheck(IEntity entity) =>
// Mobiles that are deleted, but have a reference to a corpse that is not deleted should be exempt
(entity as Mobile)?.Corpse?.Deleted == false;
}

View file

@ -13,18 +13,31 @@ namespace System;
/// </summary>
internal sealed class Gen2GcCallback : CriticalFinalizerObject
{
private readonly Func<object, bool> _callback;
private readonly Func<bool>? _callback0;
private readonly Func<object, bool>? _callback1;
private GCHandle _weakTargetObj;
private Gen2GcCallback(Func<bool> callback) => _callback0 = callback;
private Gen2GcCallback(Func<object, bool> callback, object targetObj)
{
_callback = callback;
_callback1 = callback;
_weakTargetObj = GCHandle.Alloc(targetObj, GCHandleType.Weak);
}
/// <summary>
/// Schedule 'callback' to be called in the next GC. If the callback returns true it is
/// rescheduled for the next Gen 2 GC. Otherwise the callbacks stop.
/// rescheduled for the next Gen 2 GC, otherwise the callback stops.
/// </summary>
public static void Register(Func<bool> callback)
{
// Create an unreachable object that remembers the callback function and target object.
new Gen2GcCallback(callback);
}
/// <summary>
/// Schedule 'callback' to be called in the next GC. If the callback returns true it is
/// rescheduled for the next Gen 2 GC, otherwise the callback stops.
///
/// NOTE: This callback will be kept alive until either the callback function returns false,
/// or the target object dies.
@ -51,8 +64,8 @@ internal sealed class Gen2GcCallback : CriticalFinalizerObject
// Execute the callback method.
try
{
Debug.Assert(_callback != null);
if (_callback?.Invoke(targetObj) != true)
Debug.Assert(_callback1 != null);
if (!_callback1(targetObj))
{
// If the callback returns false, this callback object is no longer needed.
_weakTargetObj.Free();
@ -63,8 +76,29 @@ internal sealed class Gen2GcCallback : CriticalFinalizerObject
{
// Ensure that we still get a chance to resurrect this object, even if the callback throws an exception.
#if DEBUG
// Except in DEBUG, as we really shouldn't be hitting any exceptions here.
throw;
// Except in DEBUG, as we really shouldn't be hitting any exceptions here.
throw;
#endif
}
}
else
{
// Execute the callback method.
try
{
Debug.Assert(_callback0 != null);
if (!_callback0())
{
// If the callback returns false, this callback object is no longer needed.
return;
}
}
catch
{
// Ensure that we still get a chance to resurrect this object, even if the callback throws an exception.
#if DEBUG
// Except in DEBUG, as we really shouldn't be hitting any exceptions here.
throw;
#endif
}
}

View file

@ -328,7 +328,7 @@ public class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropertyListEnt
public virtual TimeSpan DecayTime => DefaultDecayTime;
[CommandProperty(AccessLevel.GameMaster)]
public virtual bool Decays => Movable && Visible && Spawner == null;
public virtual bool Decays => Movable && Visible && Spawner == null;
public DateTime LastMoved { get; set; }
@ -3268,6 +3268,13 @@ public class Item : IHued, IComparable<Item>, ISpawnable, IObjectPropertyListEnt
}
}
#if TRACK_LEAKS
~Item()
{
EntityFinalizationTracker.NotifyFinalized(this);
}
#endif
public virtual void OnDelete()
{
if (Spawner != null)

View file

@ -4574,6 +4574,13 @@ public partial class Mobile : IHued, IComparable<Mobile>, ISpawnable, IObjectPro
}
}
#if TRACK_LEAKS
~Mobile()
{
EntityFinalizationTracker.NotifyFinalized(this);
}
#endif
/// <summary>
/// Overridable. Virtual event invoked before the Mobile is deleted.
/// </summary>

View file

@ -492,7 +492,12 @@ public static class World
else
{
logger.Warning($"Attempted to call World.RemoveEntity with '{entity.GetType()}'. Must be a mobile or item.");
return;
}
#if TRACK_LEAKS
EntityFinalizationTracker.TrackEntity(entity);
#endif
}
[MethodImpl(MethodImplOptions.AggressiveInlining)]

View file

@ -27,6 +27,8 @@ public partial class SchmendrickApprenticeCorpse : Corpse
_lantern = new Lantern { Movable = false, Protected = true };
_lantern.Ignite();
Owner = null;
}
private static Mobile GetOwner()
@ -152,5 +154,7 @@ public partial class SchmendrickApprenticeCorpse : Corpse
{
_lantern.Delete();
}
Owner?.Delete();
}
}

View file

@ -423,6 +423,13 @@ public abstract partial class BaseDoor : Item, ILockable, ITelekinesisable
}
}
public override void OnDelete()
{
_timer?.Stop();
_timer = null;
Link = null;
}
[AfterDeserialization]
private void AfterDeserialization()
{
@ -442,6 +449,12 @@ public abstract partial class BaseDoor : Item, ILockable, ITelekinesisable
protected override void OnTick()
{
if (_door.Deleted)
{
Stop();
return;
}
if (_door.Open && _door.IsFreeToClose())
{
_door.Open = false;

View file

@ -109,7 +109,7 @@ public partial class Corpse : Container, ICarvable
[SerializableField(7, setter: "private")]
private List<Mobile> _aggressors;
[SerializableField(8, setter: "private")]
[SerializableField(8, setter: "protected")]
[SerializedCommandProperty(AccessLevel.GameMaster)]
private Mobile _owner;

Binary file not shown.