From 59dcd24bed748216c3a17b987ef0de5479a90688 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Tue, 8 Nov 2022 09:08:05 -0800 Subject: [PATCH] fix: Fixes DeltaQueue recursion, StatMod bug, and memory leak (#1233) * Fixes recursion with DeltaQueue causing multiple sets of packets to be sent to the client. * Fixes StatMods that are expired not being checked properly. * Adds a timer to properly expire/remove stat mods instead of relying on a side-effect. --- Projects/Server/Mobiles/Mobile.cs | 54 +++++++++++-------- Projects/Server/Mobiles/Mods/StatMod.cs | 26 ++++++++- Projects/UOContent/Spells/Base/SpellHelper.cs | 7 ++- 3 files changed, 61 insertions(+), 26 deletions(-) diff --git a/Projects/Server/Mobiles/Mobile.cs b/Projects/Server/Mobiles/Mobile.cs index 9207e754f..d11a12b0d 100644 --- a/Projects/Server/Mobiles/Mobile.cs +++ b/Projects/Server/Mobiles/Mobile.cs @@ -8392,6 +8392,33 @@ public class Mobile : IHued, IComparable, ISpawnable, IObjectPropertyLis return false; } + public virtual void RemoveStatMod(StatMod mod) + { + if (mod == null) + { + return; + } + + // Remove it just in case it was orphaned somehow. + mod.Remove(); + + if (_statMods == null) + { + return; + } + + if (_statMods.Remove(mod)) + { + CheckStatTimers(); + Delta(MobileDelta.Stat | GetStatDelta(mod.Type)); + } + + if (_statMods.Count == 0) + { + _statMods = null; + } + } + public virtual void RemoveStatMod(string name) { if (_statMods == null || name == null) @@ -8405,19 +8432,19 @@ public class Mobile : IHued, IComparable, ISpawnable, IObjectPropertyLis if (mod.Name == name) { _statMods.RemoveAt(i); + mod.Remove(); CheckStatTimers(); Delta(MobileDelta.Stat | GetStatDelta(mod.Type)); } } - if (_statMods.Count == 0) { _statMods = null; } } - public virtual StatMod GetStatMod(string name) + public virtual StatMod GetStatMod(string name, bool includeElapsed = false) { if (_statMods == null || name == null) { @@ -8427,7 +8454,7 @@ public class Mobile : IHued, IComparable, ISpawnable, IObjectPropertyLis for (var i = 0; i < _statMods.Count; i++) { var mod = _statMods[i]; - if (mod.Name == name) + if (mod.Name == name && !mod.HasElapsed() || includeElapsed) { return mod; } @@ -8471,41 +8498,26 @@ public class Mobile : IHued, IComparable, ISpawnable, IObjectPropertyLis } /// - /// Computes the total modified offset for the specified stat type. Expired instances are removed. + /// Computes the total modified offset for the specified stat type. /// public int GetStatOffset(StatType type) { - _statMods ??= new List(); - - if (_statMods.Count <= 0) + if (_statMods == null || _statMods.Count == 0) { return 0; } var offset = 0; - using var queue = PooledRefQueue.Create(8); for (var i = 0; i < _statMods.Count; i++) { var mod = _statMods[i]; - if (mod.HasElapsed()) - { - queue.Enqueue(mod); - } - else if ((mod.Type & type) != 0) + if ((mod.Type & type) != 0 && !mod.HasElapsed()) { offset += mod.Offset; } } - while (queue.Count > 0) - { - var mod = queue.Dequeue(); - _statMods.Remove(mod); - Delta(MobileDelta.Stat | GetStatDelta(mod.Type)); - CheckStatTimers(); - } - return offset; } diff --git a/Projects/Server/Mobiles/Mods/StatMod.cs b/Projects/Server/Mobiles/Mods/StatMod.cs index 0475879cd..784bb8796 100644 --- a/Projects/Server/Mobiles/Mods/StatMod.cs +++ b/Projects/Server/Mobiles/Mods/StatMod.cs @@ -33,6 +33,11 @@ public partial class StatMod : MobileMod [SerializableField(3, setter: "private")] private int _offset; + // Added a timer and removed the processing of expirations in GetStatOffset which caused recursions in DeltaQueue + // ProcessDeltaQueue -> Dequeue -> ProcessDelta -> SendStats packet -> get_HitsMax -> + // - GetStatOffset -> Delta -> Queue to DeltaQueue + private TimerExecutionToken _timerToken; + public StatMod(Mobile owner) : base(owner) { } @@ -43,7 +48,26 @@ public partial class StatMod : MobileMod _offset = offset; _duration = duration; _added = Core.Now; + + if (_duration > TimeSpan.Zero) + { + Timer.StartTimer(duration, RemoveFromOwner, out _timerToken); + } } - public bool HasElapsed() => _duration != TimeSpan.Zero && Core.Now - _added >= _duration; + public bool HasElapsed() => _duration > TimeSpan.Zero && Core.Now - _added >= _duration; + + [AfterDeserialization] + private void AfterDeserialization() + { + if (_duration > TimeSpan.Zero && Core.Now - _added < _duration) + { + Timer.StartTimer(_duration, RemoveFromOwner, out _timerToken); + } + } + + private void RemoveFromOwner() => Owner?.RemoveStatMod(this); + + // Called by Mobile.RemoveStatMod() + public void Remove() => _timerToken.Cancel(); } diff --git a/Projects/UOContent/Spells/Base/SpellHelper.cs b/Projects/UOContent/Spells/Base/SpellHelper.cs index 2c891d99b..8eceb2591 100644 --- a/Projects/UOContent/Spells/Base/SpellHelper.cs +++ b/Projects/UOContent/Spells/Base/SpellHelper.cs @@ -301,20 +301,19 @@ namespace Server.Spells public static bool AddStatBonus(Mobile caster, Mobile target, StatType type, int bonus, TimeSpan duration) { - var offset = bonus; var name = $"[Magic] {type} Buff"; var mod = target.GetStatMod(name); if (mod?.Offset < 0) { - target.AddStatMod(new StatMod(type, name, mod.Offset + offset, duration)); + target.AddStatMod(new StatMod(type, name, mod.Offset + bonus, duration)); return true; } - if (mod == null || mod.Offset <= offset) + if (mod == null || mod.Offset <= bonus) { - target.AddStatMod(new StatMod(type, name, offset, duration)); + target.AddStatMod(new StatMod(type, name, bonus, duration)); return true; }