fix: Removes unnecessary dictionary removal guards

Dictionary<K,V>.Remove and HashSet<T>.Remove do not bump the collection
version, so removing during a foreach is safe and the collect-then-remove
queues guarding those loops are dead weight. This drops them, including
via the Keys and Values enumerators.

Deletion of a Mobile is left deferred: removing from World.Mobiles while
enumerating it is fine, but Delete() runs OnDelete/OnAfterDelete, the
OnParentDeleted cascade over the pack, and region and guild callbacks.
Anything in there that constructs a Mobile is an Add into the dictionary
being enumerated, which does invalidate it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Kamron Batman 2026-08-08 10:42:48 -07:00
parent f33bcd6006
commit a2fd2441ae
No known key found for this signature in database
GPG key ID: 7D81DF26D9A5D94A
19 changed files with 45 additions and 183 deletions

View file

@ -1050,28 +1050,16 @@ public static partial class Utility
return;
}
using var queue = PooledRefQueue<K>.Create();
foreach (var (key, value) in dictionary)
{
if (serializableKey)
{
if (key == null || ((ISerializable)key).Deleted)
{
queue.Enqueue(key);
}
}
else
{
if (value == null || ((ISerializable)value).Deleted)
{
queue.Enqueue(key);
}
}
}
var deleted = serializableKey
? ((ISerializable)key).Deleted
: value == null || ((ISerializable)value).Deleted;
while (queue.Count > 0)
{
dictionary.Remove(queue.Dequeue());
if (deleted)
{
dictionary.Remove(key);
}
}
dictionary.TrimExcess();

View file

@ -103,9 +103,8 @@ namespace Server.Commands
{
var list = pm.VisibilityList;
if (list.Contains(targ))
if (list.Remove(targ))
{
list.Remove(targ);
pm.SendMessage($"{targ.Name} has been removed from your visibility list.");
}
else

View file

@ -167,20 +167,13 @@ public class ChampionTitleSystem : GenericPersistence
return;
}
using var queue = PooledRefQueue<Mobile>.Create();
foreach (var context in _championTitleContexts.Values)
{
if (!context.CheckAtrophy())
{
queue.Enqueue(context.Player);
_championTitleContexts.Remove(context.Player);
}
}
while (queue.Count > 0)
{
_championTitleContexts.Remove((PlayerMobile)queue.Dequeue());
}
}
}
}

View file

@ -146,15 +146,8 @@ namespace Server.Engines.Chat
m_Users.Remove(user);
user.CurrentChannel = null;
if (m_Moderators.Contains(user))
{
m_Moderators.Remove(user);
}
if (m_Voices.Contains(user))
{
m_Voices.Remove(user);
}
m_Moderators.Remove(user);
m_Voices.Remove(user);
SendCommand(ChatCommand.RemoveUserFromChannel, user, user.Username);
ChatSystem.SendCommandTo(user.Mobile, ChatCommand.LeaveChannel);
@ -183,10 +176,7 @@ namespace Server.Engines.Chat
public void RemoveBan(ChatUser user)
{
if (m_Banned.Contains(user))
{
m_Banned.Remove(user);
}
m_Banned.Remove(user);
}
public void Kick(ChatUser user, ChatUser moderator = null)

View file

@ -716,10 +716,7 @@ public partial class BRBomb : Item
m.Target = new BombTarget(this, m);
if (m_Helpers.Contains(m))
{
m_Helpers.Remove(m);
}
m_Helpers.Remove(m);
if (m_Helpers.Count > 0)
{

View file

@ -56,12 +56,11 @@ namespace Server.Engines.ConPVP
public void RemoveFlavor(Ruleset flavor)
{
if (!Flavors.Contains(flavor))
if (!Flavors.Remove(flavor))
{
return;
}
Flavors.Remove(flavor);
Options.And(flavor.Options.Not());
flavor.Options.Not();
}

View file

@ -550,21 +550,14 @@ namespace Server.Items
return;
}
using var toDelete = PooledRefQueue<Mobile>.Create();
foreach (var (key, value) in _guesses)
{
if (Core.Now - value.When > CleanupTime)
{
toDelete.Enqueue(key);
_guesses.Remove(key);
}
}
while (toDelete.Count > 0)
{
_guesses.Remove(toDelete.Dequeue());
}
if (_guesses.Count == 0)
{
_guesses = null;

View file

@ -116,10 +116,9 @@ namespace Server.Engines.MLQuests.Gumps
private static void CloseCurrent(NetState ns)
{
if (m_Pending.TryGetValue(ns, out var state))
if (m_Pending.Remove(ns, out var state))
{
state._timeoutToken.Cancel();
m_Pending.Remove(ns);
}
ns.SendCloseRaceChanger();

View file

@ -727,20 +727,14 @@ public sealed class StepCache
var window = MissPromotionWindowMs;
var beforeCount = _chunkMissTracker.Count;
using var toRemove = PooledRefQueue<long>.Create();
foreach (var kvp in _chunkMissTracker)
{
if (now - kvp.Value.LastMissTickStamp > window)
{
toRemove.Enqueue(kvp.Key);
_chunkMissTracker.Remove(kvp.Key);
}
}
while (toRemove.Count > 0)
{
_chunkMissTracker.Remove(toRemove.Dequeue());
}
if (_chunkMissTracker.Count == beforeCount)
{
_chunkMissTracker.Clear();

View file

@ -403,27 +403,20 @@ public class PlayerMurderSystem : GenericPersistence
return;
}
using var queue = PooledRefQueue<Mobile>.Create();
foreach (var context in _contextTerms)
{
context.DecayKills();
if (!context.CheckStart())
{
queue.Enqueue(context.Player);
}
}
while (queue.Count > 0)
{
var pm = (PlayerMobile)queue.Dequeue();
if (_murderContexts.TryGetValue(pm, out var ctx))
{
if (ctx.CanRemove())
var pm = context.Player;
if (_murderContexts.TryGetValue(pm, out var ctx))
{
_murderContexts.Remove(pm);
if (ctx.CanRemove())
{
_murderContexts.Remove(pm);
}
_contextTerms.Remove(ctx);
}
_contextTerms.Remove(ctx);
}
}
}

View file

@ -372,8 +372,6 @@ public class VirtueSystem : GenericPersistence
return;
}
using var queue = PooledRefQueue<Mobile>.Create();
// This is not particularly efficient. If it gets too slow, then use a different architecture.
foreach (var (player, virtues) in _playerVirtues)
{
@ -381,14 +379,9 @@ public class VirtueSystem : GenericPersistence
if (!virtues.IsUsed())
{
queue.Enqueue(player);
_playerVirtues.Remove(player);
}
}
while (queue.Count > 0)
{
_playerVirtues.Remove((PlayerMobile)queue.Dequeue());
}
}
~VirtueTimer()

View file

@ -126,20 +126,14 @@ public partial class ProjectedItem : Item
private static void OnTick()
{
using var queue = PooledRefQueue<Item>.Create();
foreach (var item in _active)
{
if (!item.SendEffect())
{
queue.Enqueue(item);
_active.Remove(item);
}
}
while (queue.Count > 0)
{
_active.Remove(queue.Dequeue() as ProjectedItem);
}
if (_active.Count == 0)
{
_timer?.Stop();

View file

@ -3,7 +3,6 @@ using System.Collections.Generic;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using ModernUO.CodeGeneratedEvents;
using Server.Collections;
using Server.Logging;
using Server.Mobiles;
using Server.Spells.Ninjitsu;
@ -60,22 +59,16 @@ public static class StaminaSystem
EventSink.Logout += Logout;
// Credit idle time
using var queue = PooledRefQueue<IHasSteps>.Create();
foreach (var m in _stepsTaken.Keys)
{
// We cannot remove since we are iterating.
// Keeps the ref valid for the check below.
ref var stepsTaken = ref RegenSteps(m, out var exists, removeOnInvalidation: false);
if (exists && stepsTaken.Steps <= 0)
{
queue.Enqueue(m);
_stepsTaken.Remove(m);
}
}
while (queue.Count > 0)
{
_stepsTaken.Remove(queue.Dequeue());
}
}
[OnEvent(nameof(PlayerMobile.PlayerDeletedEvent))]
@ -325,7 +318,7 @@ public static class StaminaSystem
{
var from = e.Mobile;
var running = (e.Direction & Direction.Running) != 0;
if (CannotWalkWhenFatigued && from.Stam <= 0)
{
from.SendLocalizedMessage(500110); // You are too fatigued to move.
@ -481,27 +474,13 @@ public static class StaminaSystem
if (_resetHash.Count > 0)
{
using var queue = PooledRefQueue<IHasSteps>.Create();
ref var stepsTaken = ref Unsafe.NullRef<StepsTaken>();
foreach (var m in _resetHash)
{
stepsTaken = ref GetStepsTaken(m, out var exists);
if (!exists || Core.Now >= stepsTaken.IdleStartTime + ResetDuration)
{
queue.Enqueue(m);
}
}
if (_resetHash.Count == queue.Count)
{
_resetHash.Clear();
}
else
{
while (queue.Count > 0)
{
_resetHash.Remove(queue.Dequeue());
_resetHash.Remove(m);
}
}
}

View file

@ -27,20 +27,13 @@ public abstract partial class BaseAI
private static void CleanupReservedPositions()
{
using var toRemove = PooledRefQueue<BaseCreature>.Create();
foreach (var (m, p) in _reservedPositions)
{
if (m?.Deleted != false || m.GetDistanceToSqrt(p) < 1)
{
toRemove.Enqueue(m);
_reservedPositions.Remove(m);
}
}
while (toRemove.Count > 0)
{
_reservedPositions.Remove(toRemove.Dequeue());
}
}
private bool UseGroupMovement(Mobile target) =>

View file

@ -17,7 +17,6 @@ using System;
using System.Collections.Generic;
using System.Net;
using System.Threading;
using Server.Collections;
using Server.Logging;
using Server.Network.Bans;
@ -134,22 +133,18 @@ public static class AutoDenylist
return;
}
using var lapsed = new PooledRefList<UInt128>(16);
var lapsed = 0;
foreach (var (address, expires) in _held)
{
if (expires - nowTicks <= 0)
{
lapsed.Add(address);
_held.Remove(address);
lapsed++;
}
}
for (var i = 0; i < lapsed.Count; i++)
{
_held.Remove(lapsed[i]);
}
if (lapsed.Count > 0)
if (lapsed > 0)
{
_warnedFull = false;
}

View file

@ -39,17 +39,13 @@ public sealed class PromotedGuard
{
return;
}
using var dead = Collections.PooledRefQueue<UInt128>.Create();
foreach (var (ip, exp) in _expiry)
{
if (exp - nowTicks <= 0)
{
dead.Enqueue(ip);
_expiry.Remove(ip);
}
}
while (dead.Count > 0)
{
_expiry.Remove(dead.Dequeue());
}
}
}

View file

@ -20,7 +20,6 @@ using System.IO;
using System.Net;
using System.Text;
using System.Threading.Tasks;
using Server.Collections;
using Server.Logging;
using Server.Network.Bans;
@ -230,13 +229,15 @@ public static class LoginAllowlist
var stamps = new long[_allowed.Count];
var count = 0;
using var expired = new PooledRefList<UInt128>(16);
var dropped = 0;
foreach (var (address, stamp) in _allowed)
{
if (stamp < cutoff)
{
expired.Add(address);
_allowed.Remove(address);
_strikes.Remove(address);
dropped++;
continue;
}
@ -245,19 +246,12 @@ public static class LoginAllowlist
count++;
}
for (var i = 0; i < expired.Count; i++)
{
_allowed.Remove(expired[i]);
_strikes.Remove(expired[i]);
}
PruneStaleStrikes(nowUnix);
_dirty = false;
var path = _path;
var total = count;
var dropped = expired.Count;
_ = Task.Run(() => Write(path, addresses, stamps, total, dropped));
}
@ -270,20 +264,13 @@ public static class LoginAllowlist
return;
}
using var stale = new PooledRefList<UInt128>(16);
foreach (var (address, strike) in _strikes)
{
if (nowUnix - strike.WindowStart > _strikeWindowSeconds)
{
stale.Add(address);
_strikes.Remove(address);
}
}
for (var i = 0; i < stale.Count; i++)
{
_strikes.Remove(stale[i]);
}
}
private static void Write(string path, UInt128[] addresses, long[] stamps, int count, int dropped)

View file

@ -120,23 +120,17 @@ public static class AntiMacroSystem
var now = Core.Now;
using var toRemove = PooledRefQueue<Mobile>.Create();
foreach (var (m, antiMacro) in _antiMacroTable)
{
if (antiMacro._lastExpiration <= now)
{
toRemove.Enqueue(m);
_antiMacroTable.Remove(m);
}
else
{
antiMacro.CleanExpired();
}
}
while (toRemove.Count > 0)
{
_antiMacroTable.Remove(toRemove.Dequeue());
}
}
[OnEvent(nameof(PlayerMobile.PlayerLoginEvent))]
@ -259,20 +253,13 @@ public static class AntiMacroSystem
{
var now = Core.Now;
using var toRemove = PooledRefQueue<(Skill, object)>.Create();
foreach (var (key, countAndTimeStamp) in _antiMacroTracking)
{
if (countAndTimeStamp._count <= 0 || countAndTimeStamp._expiration <= now)
{
toRemove.Enqueue(key);
_antiMacroTracking.Remove(key);
}
}
while (toRemove.Count > 0)
{
_antiMacroTracking.Remove(toRemove.Dequeue());
}
}
}

View file

@ -39,20 +39,13 @@ public static class DetectHidden
// Clean up old debounce entries to prevent memory bloat
private static void CleanupDebounceCache(long now)
{
using var entriesToRemove = PooledRefQueue<(Mobile, Mobile)>.Create();
foreach (var entry in PassiveDetectDebounce)
{
if (now - entry.Value > DebounceExpiryMs)
{
entriesToRemove.Enqueue(entry.Key);
PassiveDetectDebounce.Remove(entry.Key);
}
}
while (entriesToRemove.Count > 0)
{
PassiveDetectDebounce.Remove(entriesToRemove.Dequeue());
}
}
// For testing: clear the debounce cache to prevent cross-test contamination