From 46595b139932d43724b0fb1c7e31e1ffd86919fc Mon Sep 17 00:00:00 2001 From: Eero Date: Sun, 28 Dec 2025 04:59:56 +0800 Subject: [PATCH] WIP Make collections thread-safe and add safe iteration Replaced static lists and dictionaries with thread-safe ConcurrentDictionary or ThreadLocal collections for various item components and systems. Updated all relevant code to use snapshots (ToArray, ToList) for safe iteration, and added helper methods for marking and clearing changed connections. These changes improve thread safety and prevent potential concurrency issues in multi-threaded scenarios. --- .../SharedSource/GameSession/GameSession.cs | 2 +- .../Items/Components/DockingPort.cs | 20 +- .../SharedSource/Items/Components/Door.cs | 9 +- .../Items/Components/ElectricalDischarger.cs | 12 +- .../Items/Components/Holdable/Holdable.cs | 12 +- .../Items/Components/Holdable/RepairTool.cs | 32 ++-- .../Items/Components/ItemComponent.cs | 3 +- .../Items/Components/ItemContainer.cs | 3 +- .../SharedSource/Items/Components/Ladder.cs | 10 +- .../Items/Components/Machines/Controller.cs | 11 +- .../Items/Components/Machines/Sonar.cs | 23 ++- .../Items/Components/Power/PowerTransfer.cs | 119 ++++++------ .../Items/Components/Power/Powered.cs | 108 +++++++++-- .../Items/Components/Repairable.cs | 9 +- .../Items/Components/Signal/Connection.cs | 43 +++-- .../Components/Signal/ConnectionPanel.cs | 20 +- .../Items/Components/Signal/RelayComponent.cs | 6 +- .../Items/Components/Signal/WifiComponent.cs | 15 +- .../Items/Components/Signal/Wire.cs | 3 +- .../SharedSource/Items/Inventory.cs | 3 +- .../SharedSource/Items/Item.cs | 21 ++- .../SharedSource/Map/MapEntity.cs | 4 +- .../SharedSource/Map/Submarine.cs | 4 +- .../StatusEffects/DelayedEffect.cs | 96 +++++++--- .../StatusEffects/StatusEffect.cs | 176 ++++++++++++++---- 25 files changed, 519 insertions(+), 245 deletions(-) diff --git a/Barotrauma/BarotraumaShared/SharedSource/GameSession/GameSession.cs b/Barotrauma/BarotraumaShared/SharedSource/GameSession/GameSession.cs index f3782f134..1af627a98 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/GameSession/GameSession.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/GameSession/GameSession.cs @@ -1094,7 +1094,7 @@ namespace Barotrauma #endif //Clear the grids to allow for garbage collection Powered.Grids.Clear(); - Powered.ChangedConnections.Clear(); + Powered.ClearChangedConnections(); try { diff --git a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/DockingPort.cs b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/DockingPort.cs index be801e1eb..0046999ff 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/DockingPort.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/DockingPort.cs @@ -5,6 +5,7 @@ using FarseerPhysics.Dynamics; using FarseerPhysics.Dynamics.Joints; using Microsoft.Xna.Framework; using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; #if CLIENT @@ -24,11 +25,8 @@ namespace Barotrauma.Items.Components Right } - private static readonly List list = new List(); - public static IEnumerable List - { - get { return list; } - } + private static readonly ConcurrentDictionary _dockingPortDict = new ConcurrentDictionary(); + public static IEnumerable List => _dockingPortDict.Keys; private Sprite overlaySprite; private float dockingState; @@ -168,7 +166,7 @@ namespace Barotrauma.Items.Components IsActive = true; - list.Add(this); + _dockingPortDict.TryAdd(this, 0); } public override void FlipX(bool relativeToSub) @@ -200,7 +198,7 @@ namespace Barotrauma.Items.Components { float closestDist = float.MaxValue; DockingPort closestPort = null; - foreach (DockingPort port in list) + foreach (DockingPort port in List) { if (port == this || port.item.Submarine == item.Submarine || port.IsHorizontal != IsHorizontal) { continue; } float xDist = Math.Abs(port.item.WorldPosition.X - item.WorldPosition.X); @@ -532,8 +530,8 @@ namespace Barotrauma.Items.Components wire.TryConnect(recipient, addNode: false); //Flag connections to be updated - Powered.ChangedConnections.Add(powerConnection); - Powered.ChangedConnections.Add(recipient); + Powered.MarkConnectionChanged(powerConnection); + Powered.MarkConnectionChanged(recipient); } private void CreateDoorBody() @@ -1007,7 +1005,7 @@ namespace Barotrauma.Items.Components Connection powerConnection = Item.Connections.Find(c => c.IsPower); if (powerConnection != null) { - Powered.ChangedConnections.Add(powerConnection); + Powered.MarkConnectionChanged(powerConnection); } if (doorBody != null) @@ -1151,7 +1149,7 @@ namespace Barotrauma.Items.Components protected override void RemoveComponentSpecific() { base.RemoveComponentSpecific(); - list.Remove(this); + _dockingPortDict.TryRemove(this, out _); hulls[0]?.Remove(); hulls[0] = null; hulls[1]?.Remove(); hulls[1] = null; gap?.Remove(); gap = null; diff --git a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Door.cs b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Door.cs index 65b00c67c..fad7c7fab 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Door.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Door.cs @@ -2,6 +2,7 @@ using FarseerPhysics; using Microsoft.Xna.Framework; using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; using FarseerPhysics.Dynamics; @@ -14,9 +15,9 @@ namespace Barotrauma.Items.Components { partial class Door : Pickable, IDrawableComponent, IServerSerializable { - private static readonly HashSet doorList = new HashSet(); + private static readonly ConcurrentDictionary _doorDict = new ConcurrentDictionary(); - public static IReadOnlyCollection DoorList { get { return doorList; } } + public static ICollection DoorList => _doorDict.Keys; private Gap linkedGap; private bool isOpen; @@ -277,7 +278,7 @@ namespace Barotrauma.Items.Components } IsActive = true; - doorList.Add(this); + _doorDict.TryAdd(this, 0); } public override void OnItemLoaded() @@ -669,7 +670,7 @@ namespace Barotrauma.Items.Components convexHull2?.Remove(); #endif - doorList.Remove(this); + _doorDict.TryRemove(this, out _); } private bool CheckSubmarinesInDoorWay() diff --git a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/ElectricalDischarger.cs b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/ElectricalDischarger.cs index 4bdbe2d0f..e78319bf1 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/ElectricalDischarger.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/ElectricalDischarger.cs @@ -3,6 +3,7 @@ using Barotrauma.Networking; using FarseerPhysics; using Microsoft.Xna.Framework; using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; @@ -10,11 +11,8 @@ namespace Barotrauma.Items.Components { partial class ElectricalDischarger : Powered, IServerSerializable { - private static readonly List list = new List(); - public static IEnumerable List - { - get { return list; } - } + private static readonly ConcurrentDictionary _dischargerDict = new ConcurrentDictionary(); + public static IEnumerable List => _dischargerDict.Keys; const int MaxNodes = 100; const float MaxNodeDistance = 150.0f; @@ -115,7 +113,7 @@ namespace Barotrauma.Items.Components public ElectricalDischarger(Item item, ContentXElement element) : base(item, element) { - list.Add(this); + _dischargerDict.TryAdd(this, 0); foreach (var subElement in element.Elements()) { @@ -604,7 +602,7 @@ namespace Barotrauma.Items.Components protected override void RemoveComponentSpecific() { base.RemoveComponentSpecific(); - list.Remove(this); + _dischargerDict.TryRemove(this, out _); } public void ServerEventWrite(IWriteMessage msg, Client c, NetEntityEvent.IData extraData = null) diff --git a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Holdable/Holdable.cs b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Holdable/Holdable.cs index dacf4103a..429554361 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Holdable/Holdable.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Holdable/Holdable.cs @@ -9,6 +9,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; +using System.Threading; using System.Xml.Linq; namespace Barotrauma.Items.Components @@ -616,12 +617,13 @@ namespace Barotrauma.Items.Components return CanBeAttached(user, out _); } - private static List tempOverlappingItems = new List(); + private static readonly ThreadLocal> tempOverlappingItems = new ThreadLocal>(() => new List()); private bool CanBeAttached(Character user, out IEnumerable overlappingItems) { - tempOverlappingItems.Clear(); - overlappingItems = tempOverlappingItems; + var overlapping = tempOverlappingItems.Value; + overlapping.Clear(); + overlappingItems = overlapping; if (!attachable || !Reattachable) { return false; } //can be attached anywhere in sub editor @@ -664,9 +666,9 @@ namespace Barotrauma.Items.Components } if (attachPos.X + size.X < worldRect.X || attachPos.X - size.X > worldRect.Right) { continue; } if (attachPos.Y - size.Y > worldRect.Y || attachPos.Y + size.Y < worldRect.Y - worldRect.Height) { continue; } - tempOverlappingItems.Add(otherItem); + overlapping.Add(otherItem); } - if (tempOverlappingItems.Any()) { return false; } + if (overlapping.Any()) { return false; } } //can be attached anywhere inside hulls diff --git a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Holdable/RepairTool.cs b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Holdable/RepairTool.cs index f5c73ab42..63aaf6726 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Holdable/RepairTool.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Holdable/RepairTool.cs @@ -4,6 +4,7 @@ using Microsoft.Xna.Framework; using System; using System.Collections.Generic; using System.Linq; +using System.Threading; using Barotrauma.Extensions; using Barotrauma.MapCreatures.Behavior; @@ -315,7 +316,7 @@ namespace Barotrauma.Items.Components partial void UseProjSpecific(float deltaTime, Vector2 raystart); - private static readonly List hitBodies = new List(); + private static readonly ThreadLocal> hitBodies = new ThreadLocal>(() => new List()); private readonly HashSet hitCharacters = new HashSet(); private readonly List fireSourcesInRange = new List(); private void Repair(Vector2 rayStart, Vector2 rayEnd, float deltaTime, Character user, float degreeOfSuccess, List ignoredBodies) @@ -373,13 +374,13 @@ namespace Barotrauma.Items.Components }, allowInsideFixture: true); - hitBodies.Clear(); - hitBodies.AddRange(bodies.Distinct()); + hitBodies.Value.Clear(); + hitBodies.Value.AddRange(bodies.Distinct()); lastPickedFraction = Submarine.LastPickedFraction; Type lastHitType = null; hitCharacters.Clear(); - foreach (Body body in hitBodies) + foreach (Body body in hitBodies.Value) { Type bodyType = body.UserData?.GetType(); if (!RepairThroughWalls && bodyType != null && bodyType != lastHitType) @@ -897,48 +898,49 @@ namespace Barotrauma.Items.Components } } - private static List currentTargets = new List(); + private static readonly ThreadLocal> currentTargets = new ThreadLocal>(() => new List()); private void ApplyStatusEffectsOnTarget(Character user, float deltaTime, ActionType actionType, Item targetItem = null, Character character = null, Limb limb = null, Structure structure = null) { if (statusEffectLists == null) { return; } if (!statusEffectLists.TryGetValue(actionType, out List statusEffects)) { return; } + var targets = currentTargets.Value; foreach (StatusEffect effect in statusEffects) { - currentTargets.Clear(); + targets.Clear(); effect.SetUser(user); if (effect.HasTargetType(StatusEffect.TargetType.UseTarget)) { if (targetItem != null) { - currentTargets.AddRange(targetItem.AllPropertyObjects); + targets.AddRange(targetItem.AllPropertyObjects); } if (structure != null) { - currentTargets.Add(structure); + targets.Add(structure); } if (character != null) { - currentTargets.Add(character); + targets.Add(character); } - effect.Apply(actionType, deltaTime, item, currentTargets); + effect.Apply(actionType, deltaTime, item, targets); } else if (effect.HasTargetType(StatusEffect.TargetType.Character)) { - currentTargets.Add(user); - effect.Apply(actionType, deltaTime, item, currentTargets); + targets.Add(user); + effect.Apply(actionType, deltaTime, item, targets); } else if (effect.HasTargetType(StatusEffect.TargetType.Limb)) { - currentTargets.Add(limb); - effect.Apply(actionType, deltaTime, item, currentTargets); + targets.Add(limb); + effect.Apply(actionType, deltaTime, item, targets); } #if CLIENT if (user == null) { return; } // Hard-coded progress bars for welding doors stuck. // A general purpose system could be better, but it would most likely require changes in the way we define the status effects in xml. - foreach (ISerializableEntity target in currentTargets) + foreach (ISerializableEntity target in targets) { if (target is not Door door) { continue; } if (!door.CanBeWelded || !door.Item.IsInteractable(user)) { continue; } diff --git a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/ItemComponent.cs b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/ItemComponent.cs index 8e156bb54..0ae99cd9e 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/ItemComponent.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/ItemComponent.cs @@ -949,7 +949,8 @@ namespace Barotrauma.Items.Components //if any of the effects reduce the item's condition, set the user for OnBroken effects as well if (reducesCondition && user != null && type != ActionType.OnBroken) { - foreach (ItemComponent ic in item.Components) + // Use ToArray() snapshot for thread-safe iteration + foreach (ItemComponent ic in item.Components.ToArray()) { if (ic.statusEffectLists == null || !ic.statusEffectLists.TryGetValue(ActionType.OnBroken, out List brokenEffects)) { continue; } foreach (var brokenEffect in brokenEffects) diff --git a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/ItemContainer.cs b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/ItemContainer.cs index ae7016391..722b63588 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/ItemContainer.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/ItemContainer.cs @@ -884,7 +884,8 @@ namespace Barotrauma.Items.Components RelatedItem containableItem = FindContainableItem(containedItem); if (containableItem != null && containableItem.SetActive) { - foreach (var ic in containedItem.Components) + // Use ToArray() snapshot for thread-safe iteration + foreach (var ic in containedItem.Components.ToArray()) { ic.IsActive = active; } diff --git a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Ladder.cs b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Ladder.cs index 73674fbf8..584089e75 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Ladder.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Ladder.cs @@ -1,17 +1,19 @@ -using System.Collections.Generic; +using System.Collections.Concurrent; +using System.Collections.Generic; using System.Xml.Linq; namespace Barotrauma.Items.Components { partial class Ladder : ItemComponent { - public static List List { get; } = new List(); + private static readonly ConcurrentDictionary _ladderDict = new ConcurrentDictionary(); + public static IEnumerable List => _ladderDict.Keys; public Ladder(Item item, ContentXElement element) : base(item, element) { InitProjSpecific(element); - List.Add(this); + _ladderDict.TryAdd(this, 0); } partial void InitProjSpecific(ContentXElement element); @@ -28,7 +30,7 @@ namespace Barotrauma.Items.Components { base.RemoveComponentSpecific(); RemoveProjSpecific(); - List.Remove(this); + _ladderDict.TryRemove(this, out _); } partial void RemoveProjSpecific(); diff --git a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Machines/Controller.cs b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Machines/Controller.cs index 899258e4f..9d032f85d 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Machines/Controller.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Machines/Controller.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Globalization; using System.Xml.Linq; +using System.Linq; namespace Barotrauma.Items.Components { @@ -497,12 +498,14 @@ namespace Barotrauma.Items.Components item.SendSignal(new Signal(MathHelper.ToDegrees(targetRotation).ToString("G", CultureInfo.InvariantCulture), sender: user), positionOut); - for (int i = item.LastSentSignalRecipients.Count - 1; i >= 0; i--) + // Use ToList() snapshot for thread-safe iteration + var signalRecipients = item.LastSentSignalRecipients.ToList(); + for (int i = signalRecipients.Count - 1; i >= 0; i--) { - if (item.LastSentSignalRecipients[i].Item.Condition <= 0.0f || item.LastSentSignalRecipients[i].IsPower) { continue; } - if (item.LastSentSignalRecipients[i].Item.Prefab.FocusOnSelected) + if (signalRecipients[i].Item.Condition <= 0.0f || signalRecipients[i].IsPower) { continue; } + if (signalRecipients[i].Item.Prefab.FocusOnSelected) { - return item.LastSentSignalRecipients[i].Item; + return signalRecipients[i].Item; } } diff --git a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Machines/Sonar.cs b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Machines/Sonar.cs index bfd4aaafe..44cdf4398 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Machines/Sonar.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Machines/Sonar.cs @@ -1,14 +1,17 @@ using Barotrauma.Networking; using Microsoft.Xna.Framework; using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; +using System.Threading; namespace Barotrauma.Items.Components { partial class Sonar : Powered, IServerSerializable, IClientSerializable { - public static List SonarList = new List(); + private static readonly ConcurrentDictionary _sonarDict = new ConcurrentDictionary(); + public static IEnumerable SonarList => _sonarDict.Keys; public enum Mode { @@ -169,7 +172,7 @@ namespace Barotrauma.Items.Components IsActive = true; InitProjSpecific(element); CurrentMode = Mode.Passive; - SonarList.Add(this); + _sonarDict.TryAdd(this, 0); } partial void InitProjSpecific(ContentXElement element); @@ -291,13 +294,15 @@ namespace Barotrauma.Items.Components return currentPingIndex != -1 && (character == null || characterUsable); } - private static readonly Dictionary> targetGroups = new Dictionary>(); + private static readonly ThreadLocal>> targetGroups = + new ThreadLocal>>(() => new Dictionary>()); public override bool CrewAIOperate(float deltaTime, Character character, AIObjectiveOperateItem objective) { if (currentMode == Mode.Passive || !aiPingCheckPending) { return false; } - foreach (List targetGroup in targetGroups.Values) + var groups = targetGroups.Value; + foreach (List targetGroup in groups.Values) { targetGroup.Clear(); } @@ -310,14 +315,14 @@ namespace Barotrauma.Items.Components #warning This is not the best key for a dictionary. string directionName = GetDirectionName(c.WorldPosition - item.WorldPosition).Value; - if (!targetGroups.ContainsKey(directionName)) + if (!groups.ContainsKey(directionName)) { - targetGroups.Add(directionName, new List()); + groups.Add(directionName, new List()); } - targetGroups[directionName].Add(c); + groups[directionName].Add(c); } - foreach (KeyValuePair> targetGroup in targetGroups) + foreach (KeyValuePair> targetGroup in groups) { if (!targetGroup.Value.Any()) { continue; } string dialogTag = "DialogSonarTarget"; @@ -401,7 +406,7 @@ namespace Barotrauma.Items.Components MineralClusters = null; #endif - SonarList.Remove(this); + _sonarDict.TryRemove(this, out _); } diff --git a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Power/PowerTransfer.cs b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Power/PowerTransfer.cs index c110e8b9e..f2a44513c 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Power/PowerTransfer.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Power/PowerTransfer.cs @@ -1,6 +1,7 @@ using Barotrauma.Extensions; using Microsoft.Xna.Framework; using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; using System.Xml.Linq; @@ -13,10 +14,12 @@ namespace Barotrauma.Items.Components private readonly HashSet signalConnections = new HashSet(); - private readonly Dictionary connectionDirty = new Dictionary(); + private readonly ConcurrentDictionary connectionDirty = new ConcurrentDictionary(); //a list of connections a given connection is connected to, either directly or via other power transfer components - private readonly Dictionary> connectedRecipients = new Dictionary>(); + //Uses ConcurrentDictionary as a thread-safe HashSet replacement + private readonly ConcurrentDictionary> connectedRecipients = + new ConcurrentDictionary>(); private float overloadCooldownTimer; private const float OverloadCooldown = 5.0f; @@ -132,7 +135,7 @@ namespace Barotrauma.Items.Components partial void InitProjectSpecific(XElement element); - private static readonly HashSet recipientsToRefresh = new HashSet(); + private static readonly System.Collections.Concurrent.ConcurrentDictionary _recipientsToRefresh = new System.Collections.Concurrent.ConcurrentDictionary(); public override void UpdateBroken(float deltaTime, Camera cam) { base.UpdateBroken(deltaTime, cam); @@ -144,20 +147,21 @@ namespace Barotrauma.Items.Components powerLoad = 0.0f; currPowerConsumption = 0.0f; SetAllConnectionsDirty(); - recipientsToRefresh.Clear(); - foreach (HashSet recipientList in connectedRecipients.Values) + _recipientsToRefresh.Clear(); + // Take snapshot for thread-safe iteration (no locks needed with ConcurrentDictionary) + foreach (var recipientDict in connectedRecipients.Values) { - foreach (Connection c in recipientList) + foreach (Connection c in recipientDict.Keys) { if (c.Item == item) { continue; } var recipientPowerTransfer = c.Item.GetComponent(); if (recipientPowerTransfer != null) { - recipientsToRefresh.Add(recipientPowerTransfer); + _recipientsToRefresh.TryAdd(recipientPowerTransfer, 0); } } } - foreach (PowerTransfer recipientPowerTransfer in recipientsToRefresh) + foreach (PowerTransfer recipientPowerTransfer in _recipientsToRefresh.Keys) { recipientPowerTransfer.SetAllConnectionsDirty(); recipientPowerTransfer.RefreshConnections(); @@ -304,58 +308,56 @@ namespace Barotrauma.Items.Components protected void RefreshConnections() { var connections = item.Connections; - foreach (Connection c in connections) + if (connections == null) { return; } + + // Take a snapshot of connections for thread-safe iteration + var connectionSnapshot = connections.ToList(); + foreach (Connection c in connectionSnapshot) { - if (!connectionDirty.ContainsKey(c)) + if (!connectionDirty.TryGetValue(c, out bool isDirty)) { connectionDirty[c] = true; + isDirty = true; } - else if (!connectionDirty[c]) + + if (!isDirty) { continue; } //find all connections that are connected to this one (directly or via another PowerTransfer) - HashSet tempConnected; - if (!connectedRecipients.ContainsKey(c)) + var tempConnected = connectedRecipients.GetOrAdd(c, _ => new ConcurrentDictionary()); + + // Get previous recipients and clear + var previousRecipients = tempConnected.Keys.ToList(); + tempConnected.Clear(); + + //mark all previous recipients as dirty + foreach (Connection recipient in previousRecipients) { - tempConnected = new HashSet(); - connectedRecipients.Add(c, tempConnected); - } - else - { - tempConnected = connectedRecipients[c]; - tempConnected.Clear(); - //mark all previous recipients as dirty - foreach (Connection recipient in tempConnected) - { - var pt = recipient.Item.GetComponent(); - if (pt != null) { pt.connectionDirty[recipient] = true; } - } + var pt = recipient.Item.GetComponent(); + if (pt != null) { pt.connectionDirty[recipient] = true; } } - tempConnected.Add(c); + tempConnected.TryAdd(c, 0); if (item.Condition > 0.0f) { GetConnected(c, tempConnected); //go through all the PowerTransfers that we're connected to and set their connections to match the ones we just calculated //(no need to go through the recursive GetConnected method again) - foreach (Connection recipient in tempConnected) + // Take snapshot for thread-safe iteration (no locks needed) + var tempConnectedSnapshot = tempConnected.Keys.ToList(); + foreach (Connection recipient in tempConnectedSnapshot) { if (recipient == c) { continue; } var recipientPowerTransfer = recipient.Item.GetComponent(); if (recipientPowerTransfer == null) { continue; } - if (!recipientPowerTransfer.connectedRecipients.ContainsKey(recipient)) + + var recipientSet = recipientPowerTransfer.connectedRecipients.GetOrAdd(recipient, _ => new ConcurrentDictionary()); + recipientSet.Clear(); + foreach (var connection in tempConnectedSnapshot) { - recipientPowerTransfer.connectedRecipients.Add(recipient, new HashSet()); - } - else - { - recipientPowerTransfer.connectedRecipients[recipient].Clear(); - } - foreach (var connection in tempConnected) - { - recipientPowerTransfer.connectedRecipients[recipient].Add(connection); + recipientSet.TryAdd(connection, 0); } recipientPowerTransfer.connectionDirty[recipient] = false; } @@ -364,19 +366,20 @@ namespace Barotrauma.Items.Components } } - //Finds all the connections that can receive a signal sent into the given connection and stores them in the hashset. - private void GetConnected(Connection c, HashSet connected) + //Finds all the connections that can receive a signal sent into the given connection and stores them in the concurrent dictionary. + private void GetConnected(Connection c, ConcurrentDictionary connected) { - var recipients = c.Recipients; + // Take snapshot for thread-safe iteration + var recipients = c.Recipients.ToList(); foreach (Connection recipient in recipients) { - if (recipient == null || connected.Contains(recipient)) { continue; } + if (recipient == null || connected.ContainsKey(recipient)) { continue; } Item it = recipient.Item; if (it == null || it.Condition <= 0.0f) { continue; } - connected.Add(recipient); + connected.TryAdd(recipient, 0); var powerTransfer = it.GetComponent(); if (powerTransfer != null && powerTransfer.CanTransfer && powerTransfer.IsActive) @@ -394,10 +397,14 @@ namespace Barotrauma.Items.Components connectionDirty[c] = true; if (c.IsPower) { - ChangedConnections.Add(c); + MarkConnectionChanged(c); if (connectedRecipients.TryGetValue(c, out var recipients)) { - recipients.Where(c => c.IsPower).ForEach(c => ChangedConnections.Add(c)); + // No lock needed - ConcurrentDictionary.Keys is thread-safe + foreach (var conn in recipients.Keys.Where(conn => conn.IsPower)) + { + MarkConnectionChanged(conn); + } } } } @@ -410,10 +417,14 @@ namespace Barotrauma.Items.Components connectionDirty[connection] = true; if (connection.IsPower) { - ChangedConnections.Add(connection); + MarkConnectionChanged(connection); if (connectedRecipients.TryGetValue(connection, out var recipients)) { - recipients.Where(c => c.IsPower).ForEach(c => ChangedConnections.Add(c)); + // No lock needed - ConcurrentDictionary.Keys is thread-safe + foreach (var conn in recipients.Keys.Where(conn => conn.IsPower)) + { + MarkConnectionChanged(conn); + } } } } @@ -452,16 +463,19 @@ namespace Barotrauma.Items.Components public override void ReceiveSignal(Signal signal, Connection connection) { if (item.Condition <= 0.0f || connection.IsPower) { return; } - if (!connectedRecipients.ContainsKey(connection)) { return; } + if (!connectedRecipients.TryGetValue(connection, out var recipients)) { return; } if (!signalConnections.Contains(connection)) { return; } - foreach (Connection recipient in connectedRecipients[connection]) + // No lock needed - ConcurrentDictionary.Keys is thread-safe + // Use ToList() snapshot for thread-safe iteration + foreach (Connection recipient in recipients.Keys.ToList()) { if (recipient.Item == item || recipient.Item == signal.source) { continue; } signal.source?.LastSentSignalRecipients.Add(recipient); - foreach (ItemComponent ic in recipient.Item.Components) + // Use ToArray() snapshot for thread-safe iteration + foreach (ItemComponent ic in recipient.Item.Components.ToArray()) { //other junction boxes don't need to receive the signal in the pass-through signal connections //because we relay it straight to the connected items without going through the whole chain of junction boxes @@ -471,7 +485,8 @@ namespace Barotrauma.Items.Components if (recipient.Effects != null && signal.value != "0" && !string.IsNullOrEmpty(signal.value)) { - foreach (StatusEffect effect in recipient.Effects) + // Use ToArray() snapshot for thread-safe iteration + foreach (StatusEffect effect in recipient.Effects.ToArray()) { recipient.Item.ApplyStatusEffect(effect, ActionType.OnUse, 1.0f); } @@ -484,7 +499,7 @@ namespace Barotrauma.Items.Components base.RemoveComponentSpecific(); connectedRecipients?.Clear(); connectionDirty?.Clear(); - recipientsToRefresh.Clear(); + _recipientsToRefresh.Clear(); } } } diff --git a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Power/Powered.cs b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Power/Powered.cs index 962b6a132..8d14850c0 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Power/Powered.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Power/Powered.cs @@ -1,4 +1,6 @@ using System; +using System.Collections.Concurrent; +using System.Threading; using Microsoft.Xna.Framework; using System.Collections.Generic; using System.Linq; @@ -62,17 +64,77 @@ namespace Barotrauma.Items.Components protected const float UpdateInterval = (float)Timing.Step; /// - /// List of all powered ItemComponents + /// List of all powered ItemComponents (thread-safe) /// - private static readonly List poweredList = new List(); + private static readonly ConcurrentDictionary _poweredDict = new ConcurrentDictionary(); + + /// + /// Cached list for iteration - updated when collection changes + /// + private static volatile List _cachedPoweredList; + private static int _poweredListVersion; + public static IEnumerable PoweredList { - get { return poweredList; } + get + { + var cached = _cachedPoweredList; + if (cached != null) return cached; + return GetCachedPoweredList(); + } + } + + private static List GetCachedPoweredList() + { + var newList = _poweredDict.Keys.ToList(); + _cachedPoweredList = newList; + return newList; + } + + private static void InvalidatePoweredListCache() + { + _cachedPoweredList = null; + Interlocked.Increment(ref _poweredListVersion); } - public static readonly HashSet ChangedConnections = new HashSet(); + /// + /// Thread-safe set of changed connections + /// + private static readonly ConcurrentDictionary _changedConnections = new ConcurrentDictionary(); + + /// + /// Gets all changed connections (snapshot) + /// + public static ICollection ChangedConnections => _changedConnections.Keys; + + /// + /// Add a connection to the changed set + /// + public static void MarkConnectionChanged(Connection c) + { + _changedConnections.TryAdd(c, 0); + } + + /// + /// Clear all changed connections + /// + public static void ClearChangedConnections() + { + _changedConnections.Clear(); + } + + /// + /// Remove a connection from the changed set + /// + public static void UnmarkConnectionChanged(Connection c) + { + _changedConnections.TryRemove(c, out _); + } - public readonly static Dictionary Grids = new Dictionary(); + /// + /// Thread-safe grid dictionary + /// + public readonly static ConcurrentDictionary Grids = new ConcurrentDictionary(); /// /// The amount of power currently consumed by the item. Negative values mean that the item is providing power to connected items @@ -209,7 +271,8 @@ namespace Barotrauma.Items.Components public Powered(Item item, ContentXElement element) : base(item, element) { - poweredList.Add(this); + _poweredDict.TryAdd(this, 0); + InvalidatePoweredListCache(); InitProjectSpecific(element); } @@ -322,17 +385,20 @@ namespace Barotrauma.Items.Components //don't use cache if there are no existing grids if (Grids.Count > 0 && useCache) { + // Take a snapshot of changed connections for iteration + var changedSnapshot = ChangedConnections.ToList(); + //delete all grids that were affected - foreach (Connection c in ChangedConnections) + foreach (Connection c in changedSnapshot) { if (c.Grid != null) { - Grids.Remove(c.Grid.ID); + Grids.TryRemove(c.Grid.ID, out _); c.Grid = null; } } - foreach (Connection c in ChangedConnections) + foreach (Connection c in changedSnapshot) { //Make sure the connection grid hasn't been resolved by another connection update //Ensure the connection has other connections @@ -346,7 +412,7 @@ namespace Barotrauma.Items.Components else { //Clear all grid IDs from connections - foreach (Powered powered in poweredList) + foreach (Powered powered in PoweredList) { //Only check devices with connectors if (powered.powerIn != null) @@ -361,7 +427,7 @@ namespace Barotrauma.Items.Components Grids.Clear(); - foreach (Powered powered in poweredList) + foreach (Powered powered in PoweredList) { if (powered.Item.Condition <= 0f) { continue; } @@ -392,7 +458,7 @@ namespace Barotrauma.Items.Components } //Clear changed connections after each update - ChangedConnections.Clear(); + ClearChangedConnections(); } private static GridInfo PropagateGrid(Connection conn) @@ -422,8 +488,8 @@ namespace Barotrauma.Items.Components c.Grid = grid; grid.AddConnection(c); - //Add on recipients - foreach (Connection otherC in c.Recipients) + //Add on recipients - use ToList() snapshot for thread-safe iteration + foreach (Connection otherC in c.Recipients.ToList()) { //Only add valid connections if (otherC.Grid != grid && (otherC.Grid == null || !Grids.ContainsKey(otherC.Grid.ID)) && ValidPowerConnection(c, otherC)) @@ -494,7 +560,7 @@ namespace Barotrauma.Items.Components } //Determine if devices are adding a load or providing power, also resolve solo nodes - foreach (Powered powered in poweredList) + foreach (Powered powered in PoweredList) { //Make voltage decay to ensure the device powers down. //This only effects devices with no power input (whose voltage is set by other means, e.g. status effects from a contained battery) @@ -730,7 +796,8 @@ namespace Barotrauma.Items.Components { if (item.Connections != null && powerIn != null) { - foreach (Connection recipient in powerIn.Recipients) + // Use ToList() snapshot for thread-safe iteration + foreach (Connection recipient in powerIn.Recipients.ToList()) { if (!recipient.IsPower || !recipient.IsOutput) { continue; } if (recipient.Item?.GetComponent() is PowerContainer battery) @@ -750,13 +817,14 @@ namespace Barotrauma.Items.Components { if (c.IsPower && c.Grid != null) { - ChangedConnections.Add(c); + MarkConnectionChanged(c); } } } base.RemoveComponentSpecific(); - poweredList.Remove(this); + _poweredDict.TryRemove(this, out _); + InvalidatePoweredListCache(); } } @@ -780,9 +848,9 @@ namespace Barotrauma.Items.Components Connections.Remove(c); //Remove the grid if it has no devices - if (Connections.Count == 0 && Powered.Grids.ContainsKey(ID)) + if (Connections.Count == 0) { - Powered.Grids.Remove(ID); + Powered.Grids.TryRemove(ID, out _); } } diff --git a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Repairable.cs b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Repairable.cs index 1aebf8187..d2c7d4525 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Repairable.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Repairable.cs @@ -456,7 +456,8 @@ namespace Barotrauma.Items.Components item.SendSignal(conditionSignal, "condition_out"); - foreach (var component in item.Components) + // Use ToArray() snapshot for thread-safe iteration + foreach (var component in item.Components.ToArray()) { if (component is IDeteriorateUnderStress deteriorateUnderStress) { @@ -713,7 +714,8 @@ namespace Barotrauma.Items.Components #endif if (LastActiveTime > Timing.TotalTime) { return true; } - foreach (ItemComponent ic in item.Components) + // Use ToArray() snapshot for thread-safe iteration + foreach (ItemComponent ic in item.Components.ToArray()) { if (ic is Fabricator || ic is Deconstructor) { @@ -761,7 +763,8 @@ namespace Barotrauma.Items.Components private float GetDeteriorationDelayMultiplier() { - foreach (ItemComponent ic in item.Components) + // Use ToArray() snapshot for thread-safe iteration + foreach (ItemComponent ic in item.Components.ToArray()) { if (ic is Engine engine) { diff --git a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Signal/Connection.cs b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Signal/Connection.cs index e14929804..d55150237 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Signal/Connection.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Signal/Connection.cs @@ -222,13 +222,14 @@ namespace Barotrauma.Items.Components public void SetRecipientsDirty() { recipientsDirty = true; - if (IsPower) { Powered.ChangedConnections.Add(this); } + if (IsPower) { Powered.MarkConnectionChanged(this); } } private void RefreshRecipients() { recipients.Clear(); - foreach (var wire in wires) + // Use ToArray() snapshot for thread-safe iteration + foreach (var wire in wires.ToArray()) { Connection recipient = wire.OtherConnection(this); if (recipient != null) { recipients.Add(recipient); } @@ -267,8 +268,8 @@ namespace Barotrauma.Items.Components //Check if both connections belong to a larger grid if (prevOtherConnection.recipients.Count > 1 && recipients.Count > 1) { - Powered.ChangedConnections.Add(prevOtherConnection); - Powered.ChangedConnections.Add(this); + Powered.MarkConnectionChanged(prevOtherConnection); + Powered.MarkConnectionChanged(this); } else if (recipients.Count > 1) { @@ -284,7 +285,7 @@ namespace Barotrauma.Items.Components else if (Grid.Connections.Count == 2) { //Delete the grid as these were the only 2 devices - Powered.Grids.Remove(Grid.ID); + Powered.Grids.TryRemove(Grid.ID, out _); Grid = null; prevOtherConnection.Grid = null; } @@ -325,8 +326,8 @@ namespace Barotrauma.Items.Components else { //Flag change so that proper grids can be formed - Powered.ChangedConnections.Add(this); - Powered.ChangedConnections.Add(otherConnection); + Powered.MarkConnectionChanged(this); + Powered.MarkConnectionChanged(otherConnection); } } @@ -339,7 +340,8 @@ namespace Barotrauma.Items.Components { LastSentSignal = signal; enumeratingWires = true; - foreach (var wire in wires) + // Use ToArray() snapshot for thread-safe iteration + foreach (var wire in wires.ToArray()) { Connection recipient = wire.OtherConnection(this); if (recipient == null) { continue; } @@ -354,14 +356,14 @@ namespace Barotrauma.Items.Components GameMain.LuaCs.Hook.Call("signalReceived." + recipient.item.Prefab.Identifier, signal, recipient); } - foreach (CircuitBoxConnection connection in CircuitBoxConnections) + foreach (CircuitBoxConnection connection in CircuitBoxConnections.ToArray()) { connection.ReceiveSignal(signal); GameMain.LuaCs.Hook.Call("signalReceived", signal, connection.Connection); GameMain.LuaCs.Hook.Call("signalReceived." + connection.Connection.Item.Prefab.Identifier, signal, connection); } enumeratingWires = false; - foreach (var removedWire in removedWires) + foreach (var removedWire in removedWires.ToArray()) { wires.Remove(removedWire); } @@ -372,14 +374,16 @@ namespace Barotrauma.Items.Components { conn.LastReceivedSignal = signal; - foreach (ItemComponent ic in conn.item.Components) + // Use ToArray() snapshot for thread-safe iteration + foreach (ItemComponent ic in conn.item.Components.ToArray()) { ic.ReceiveSignal(signal, conn); } if (conn.Effects == null || signal.value == "0") { return; } - foreach (StatusEffect effect in conn.Effects) + // Use ToArray() snapshot for thread-safe iteration + foreach (StatusEffect effect in conn.Effects.ToArray()) { conn.Item.ApplyStatusEffect(effect, ActionType.OnUse, (float)Timing.Step); } @@ -389,13 +393,15 @@ namespace Barotrauma.Items.Components { if (IsPower && Grid != null) { - Powered.ChangedConnections.Add(this); - foreach (Connection c in recipients) + Powered.MarkConnectionChanged(this); + // Use ToArray() snapshot for thread-safe iteration + foreach (Connection c in recipients.ToArray()) { - Powered.ChangedConnections.Add(c); + Powered.MarkConnectionChanged(c); } } - foreach (var wire in wires) + // Use ToArray() snapshot for thread-safe iteration + foreach (var wire in wires.ToArray()) { wire.RemoveConnection(this); recipientsDirty = true; @@ -403,7 +409,7 @@ namespace Barotrauma.Items.Components if (enumeratingWires) { - foreach (var wire in wires) + foreach (var wire in wires.ToArray()) { removedWires.Add(wire); } @@ -446,7 +452,8 @@ namespace Barotrauma.Items.Components { XElement newElement = new XElement(IsOutput ? "output" : "input", new XAttribute("name", Name)); - foreach (var wire in wires.OrderBy(w => w.Item.ID)) + // Use ToArray() snapshot before OrderBy for thread-safe iteration + foreach (var wire in wires.ToArray().OrderBy(w => w.Item.ID)) { newElement.Add(new XElement("link", new XAttribute("w", wire.Item.ID.ToString()), diff --git a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Signal/ConnectionPanel.cs b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Signal/ConnectionPanel.cs index 5fddc96c6..e25edc896 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Signal/ConnectionPanel.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Signal/ConnectionPanel.cs @@ -148,14 +148,16 @@ namespace Barotrauma.Items.Components Vector2 wireNodeOffset = item.Submarine == null ? Vector2.Zero : item.Submarine.HiddenSubPosition + amount; foreach (Connection c in Connections) { - foreach (Wire wire in c.Wires) + // Use ToArray() snapshot for thread-safe iteration + foreach (Wire wire in c.Wires.ToArray()) { if (wire == null) { continue; } TryMoveWire(wire); } } - foreach (var wire in DisconnectedWires) + // Use ToList() snapshot for thread-safe iteration + foreach (var wire in DisconnectedWires.ToList()) { TryMoveWire(wire); } @@ -387,7 +389,7 @@ namespace Barotrauma.Items.Components } foreach (var connection in Connections) { - Powered.ChangedConnections.Remove(connection); + Powered.UnmarkConnectionChanged(connection); connection.Recipients.Clear(); } Connections.Clear(); @@ -412,15 +414,19 @@ namespace Barotrauma.Items.Components msg.WriteByte((byte)Connections.Count); foreach (Connection connection in Connections) { - msg.WriteVariableUInt32((uint)connection.Wires.Count); - foreach (Wire wire in connection.Wires) + // Use ToArray() snapshot for thread-safe iteration + var wiresSnapshot = connection.Wires.ToArray(); + msg.WriteVariableUInt32((uint)wiresSnapshot.Length); + foreach (Wire wire in wiresSnapshot) { msg.WriteUInt16(wire?.Item == null ? (ushort)0 : wire.Item.ID); } } - msg.WriteUInt16((ushort)DisconnectedWires.Count); - foreach (Wire disconnectedWire in DisconnectedWires) + // Use ToList() snapshot for thread-safe iteration + var disconnectedSnapshot = DisconnectedWires.ToList(); + msg.WriteUInt16((ushort)disconnectedSnapshot.Count); + foreach (Wire disconnectedWire in disconnectedSnapshot) { msg.WriteUInt16(disconnectedWire.Item.ID); } diff --git a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Signal/RelayComponent.cs b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Signal/RelayComponent.cs index ea7c81e90..adb51a1ff 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Signal/RelayComponent.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Signal/RelayComponent.cs @@ -2,6 +2,7 @@ using Microsoft.Xna.Framework; using System; using System.Collections.Generic; +using System.Collections.Immutable; using System.Linq; using System.Xml.Linq; @@ -31,7 +32,8 @@ namespace Barotrauma.Items.Components private float thirdInverseMax = 0, loadEqnConstant = 0; - private static readonly Dictionary connectionPairs = new Dictionary + // Thread-safe immutable dictionary for connection pairs (read-only after initialization) + private static readonly ImmutableDictionary connectionPairs = new Dictionary { { "power_in", "power_out"}, { "signal_in", "signal_out" }, @@ -40,7 +42,7 @@ namespace Barotrauma.Items.Components { "signal_in3", "signal_out3" }, { "signal_in4", "signal_out4" }, { "signal_in5", "signal_out5" } - }; + }.ToImmutableDictionary(); protected override PowerPriority Priority { get { return PowerPriority.Relay; } } diff --git a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Signal/WifiComponent.cs b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Signal/WifiComponent.cs index e59ab6599..68a978626 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Signal/WifiComponent.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Signal/WifiComponent.cs @@ -1,6 +1,7 @@ using Barotrauma.Networking; using Microsoft.Xna.Framework; using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Globalization; using System.Linq; @@ -10,7 +11,8 @@ namespace Barotrauma.Items.Components { partial class WifiComponent : ItemComponent, IServerSerializable, IClientSerializable { - private static readonly List list = new List(); + private static readonly ConcurrentDictionary _wifiDict = new ConcurrentDictionary(); + private static IEnumerable AllWifiComponents => _wifiDict.Keys; const int ChannelMemorySize = 10; @@ -111,7 +113,7 @@ namespace Barotrauma.Items.Components public WifiComponent(Item item, ContentXElement element) : base (item, element) { - list.Add(this); + _wifiDict.TryAdd(this, 0); IsActive = true; } @@ -156,7 +158,7 @@ namespace Barotrauma.Items.Components /// public IEnumerable GetReceiversInRange() { - return list.Where(w => w != this && w.CanReceive(this)); + return AllWifiComponents.Where(w => w != this && w.CanReceive(this)); } public bool CanReceive(WifiComponent sender) @@ -185,7 +187,7 @@ namespace Barotrauma.Items.Components /// public IEnumerable GetTransmittersInRange() { - return list.Where(w => w != this && w.CanTransmit(this)); + return AllWifiComponents.Where(w => w != this && w.CanTransmit(this)); } public bool CanTransmit(WifiComponent sender) @@ -275,7 +277,8 @@ namespace Barotrauma.Items.Components if (signal.source != null) { - foreach (Connection receiver in wifiComp.item.LastSentSignalRecipients) + // Use ToList() snapshot for thread-safe iteration + foreach (Connection receiver in wifiComp.item.LastSentSignalRecipients.ToList()) { if (!signal.source.LastSentSignalRecipients.Contains(receiver)) { @@ -366,7 +369,7 @@ namespace Barotrauma.Items.Components protected override void RemoveComponentSpecific() { base.RemoveComponentSpecific(); - list.Remove(this); + _wifiDict.TryRemove(this, out _); } public override XElement Save(XElement parentElement) diff --git a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Signal/Wire.cs b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Signal/Wire.cs index a7d007824..2db146e99 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Signal/Wire.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Items/Components/Signal/Wire.cs @@ -250,7 +250,8 @@ namespace Barotrauma.Items.Components { if (connections[0] != null && connections[1] != null) { - foreach (ItemComponent ic in item.Components) + // Use ToArray() snapshot for thread-safe iteration + foreach (ItemComponent ic in item.Components.ToArray()) { if (ic == this) { continue; } diff --git a/Barotrauma/BarotraumaShared/SharedSource/Items/Inventory.cs b/Barotrauma/BarotraumaShared/SharedSource/Items/Inventory.cs index d1be5145e..68c35aab3 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Items/Inventory.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Items/Inventory.cs @@ -381,7 +381,8 @@ namespace Barotrauma if (Owner is not Item it) { return; } - foreach (var c in it.Components) + // Use ToArray() snapshot for thread-safe iteration + foreach (var c in it.Components.ToArray()) { c.OnInventoryChanged(); } diff --git a/Barotrauma/BarotraumaShared/SharedSource/Items/Item.cs b/Barotrauma/BarotraumaShared/SharedSource/Items/Item.cs index ed5916b09..c2576bb48 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Items/Item.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Items/Item.cs @@ -2441,10 +2441,11 @@ namespace Barotrauma { if (c.IsPower) { - Powered.ChangedConnections.Add(c); - foreach (Connection conn in c.Recipients) + Powered.MarkConnectionChanged(c); + // Use ToList() snapshot for thread-safe iteration + foreach (Connection conn in c.Recipients.ToList()) { - Powered.ChangedConnections.Add(conn); + Powered.MarkConnectionChanged(conn); } } } @@ -2980,7 +2981,8 @@ namespace Barotrauma foreach (Connection c in connectionPanel.Connections) { if (connectionFilter != null && !connectionFilter(c)) { continue; } - foreach (Connection recipient in c.Recipients) + // Use ToList() snapshot for thread-safe iteration + foreach (Connection recipient in c.Recipients.ToList()) { var component = recipient.Item.GetComponent(); if (component != null) @@ -3013,7 +3015,8 @@ namespace Barotrauma foreach (Connection c in connectionPanel.Connections) { if (connectionFilter != null && !connectionFilter(c)) { continue; } - foreach (Connection recipient in c.Recipients) + // Use ToList() snapshot for thread-safe iteration + foreach (Connection recipient in c.Recipients.ToList()) { var component = recipient.Item.GetComponent(); if (component != null && !connectedComponents.Contains(component)) @@ -3067,12 +3070,13 @@ namespace Barotrauma alreadySearched.Add(c); static IEnumerable GetRecipients(Connection c) { - foreach (Connection recipient in c.Recipients) + // Use ToList() snapshot for thread-safe iteration + foreach (Connection recipient in c.Recipients.ToList()) { yield return recipient; } //check circuit box inputs/outputs this connection is connected to - foreach (var circuitBoxConnection in c.CircuitBoxConnections) + foreach (var circuitBoxConnection in c.CircuitBoxConnections.ToArray()) { yield return circuitBoxConnection.Connection; } @@ -3212,7 +3216,8 @@ namespace Barotrauma if (signal.stepsTaken > 5 && signal.source != null) { int duplicateRecipients = 0; - foreach (var recipient in signal.source.LastSentSignalRecipients) + // Use ToList() snapshot for thread-safe iteration + foreach (var recipient in signal.source.LastSentSignalRecipients.ToList()) { if (recipient == connection) { diff --git a/Barotrauma/BarotraumaShared/SharedSource/Map/MapEntity.cs b/Barotrauma/BarotraumaShared/SharedSource/Map/MapEntity.cs index 2634f9560..8c839934b 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Map/MapEntity.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Map/MapEntity.cs @@ -712,11 +712,11 @@ namespace Barotrauma try { - foreach (Item item in itemList) + Parallel.ForEach(itemList, parallelOptions, item => { lastUpdatedItem = item; item.Update(scaledDeltaTime, cam); - } + }); } catch (InvalidOperationException e) { diff --git a/Barotrauma/BarotraumaShared/SharedSource/Map/Submarine.cs b/Barotrauma/BarotraumaShared/SharedSource/Map/Submarine.cs index 969eca15a..dfe6eceb1 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Map/Submarine.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Map/Submarine.cs @@ -2157,7 +2157,7 @@ namespace Barotrauma GameMain.World = null; Powered.Grids.Clear(); - Powered.ChangedConnections.Clear(); + Powered.ClearChangedConnections(); GC.Collect(); @@ -2197,7 +2197,7 @@ namespace Barotrauma ConnectedDockingPorts?.Clear(); - Powered.ChangedConnections.Clear(); + Powered.ClearChangedConnections(); Powered.Grids.Clear(); loaded.Remove(this); diff --git a/Barotrauma/BarotraumaShared/SharedSource/StatusEffects/DelayedEffect.cs b/Barotrauma/BarotraumaShared/SharedSource/StatusEffects/DelayedEffect.cs index 9c9be6e95..b589b8d83 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/StatusEffects/DelayedEffect.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/StatusEffects/DelayedEffect.cs @@ -1,26 +1,56 @@ using Barotrauma.Items.Components; using Microsoft.Xna.Framework; using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; +using System.Threading; namespace Barotrauma { class DelayedListElement { + public readonly long Id; public readonly DelayedEffect Parent; public readonly Entity Entity; - public Vector2? WorldPosition; + + private Vector2? _worldPosition; + private readonly object _worldPositionLock = new object(); + public Vector2? WorldPosition + { + get + { + lock (_worldPositionLock) + { + return _worldPosition; + } + } + set + { + lock (_worldPositionLock) + { + _worldPosition = value; + } + } + } + /// /// Should the delayed effect attempt to determine the position of the effect based on the targets, or just use the position that was passed to the constructor. /// public bool GetPositionBasedOnTargets; public readonly Vector2? StartPosition; public readonly List Targets; - public float Delay; + + private volatile float _delay; + public float Delay + { + get => _delay; + set => _delay = value; + } public DelayedListElement(DelayedEffect parentEffect, Entity parentEntity, IEnumerable targets, float delay, Vector2? worldPosition, Vector2? startPosition) { + Id = Interlocked.Increment(ref DelayedEffect._delayElementIdCounter); Parent = parentEffect; Entity = parentEntity; Targets = new List(targets); @@ -29,9 +59,19 @@ namespace Barotrauma StartPosition = startPosition; } } + class DelayedEffect : StatusEffect { - public static readonly List DelayList = new List(); + // Thread-safe counter for generating unique IDs for DelayedListElement + internal static long _delayElementIdCounter; + + // Thread-safe dictionary for delayed effects + public static readonly ConcurrentDictionary DelayListDict = new ConcurrentDictionary(); + + /// + /// Provides a thread-safe enumerable view of the delay list for iteration. + /// + public static IEnumerable DelayList => DelayListDict.Values; private enum DelayTypes { @@ -62,9 +102,10 @@ namespace Barotrauma if (this.type != type || !HasRequiredItems(entity)) { return; } if (!Stackable) { - foreach (var existingEffect in DelayList) + // Thread-safe iteration over ConcurrentDictionary + foreach (var kvp in DelayListDict) { - if (existingEffect.Parent == this && existingEffect.Targets.FirstOrDefault() == target) + if (kvp.Value.Parent == this && kvp.Value.Targets.FirstOrDefault() == target) { return; } @@ -72,18 +113,19 @@ namespace Barotrauma } if (!IsValidTarget(target)) { return; } - currentTargets.Clear(); - currentTargets.Add(target); - if (!HasRequiredConditions(currentTargets)) { return; } + var targets = CurrentTargets; + targets.Clear(); + targets.Add(target); + if (!HasRequiredConditions(targets)) { return; } switch (delayType) { case DelayTypes.Timer: - var newDelayListElement = new DelayedListElement(this, entity, currentTargets, delay, worldPosition ?? GetPosition(entity, currentTargets, worldPosition), startPosition: null) + var newDelayListElement = new DelayedListElement(this, entity, targets, delay, worldPosition ?? GetPosition(entity, targets, worldPosition), startPosition: null) { GetPositionBasedOnTargets = worldPosition == null }; - DelayList.Add(newDelayListElement); + DelayListDict.TryAdd(newDelayListElement.Id, newDelayListElement); break; case DelayTypes.ReachCursor: Projectile projectile = (entity as Item)?.GetComponent(); @@ -105,7 +147,8 @@ namespace Barotrauma return; } - DelayList.Add(new DelayedListElement(this, entity, currentTargets, Vector2.Distance(entity.WorldPosition, projectile.User.CursorWorldPosition), worldPosition, entity.WorldPosition)); + var reachCursorElement = new DelayedListElement(this, entity, targets, Vector2.Distance(entity.WorldPosition, projectile.User.CursorWorldPosition), worldPosition, entity.WorldPosition); + DelayListDict.TryAdd(reachCursorElement.Id, reachCursorElement); break; } } @@ -119,25 +162,28 @@ namespace Barotrauma if (delayType == DelayTypes.ReachCursor && Character.Controlled == null) { return; } if (!Stackable) { - foreach (var existingEffect in DelayList) + // Thread-safe iteration over ConcurrentDictionary + foreach (var kvp in DelayListDict) { - if (existingEffect.Parent == this && existingEffect.Targets.SequenceEqual(targets)) { return; } + if (kvp.Value.Parent == this && kvp.Value.Targets.SequenceEqual(targets)) { return; } } } - currentTargets.Clear(); + var localTargets = CurrentTargets; + localTargets.Clear(); foreach (ISerializableEntity target in targets) { if (!IsValidTarget(target)) { continue; } - currentTargets.Add(target); + localTargets.Add(target); } - if (!HasRequiredConditions(currentTargets)) { return; } + if (!HasRequiredConditions(localTargets)) { return; } switch (delayType) { case DelayTypes.Timer: - DelayList.Add(new DelayedListElement(this, entity, currentTargets, delay, worldPosition, null)); + var timerElement = new DelayedListElement(this, entity, localTargets, delay, worldPosition, null); + DelayListDict.TryAdd(timerElement.Id, timerElement); break; case DelayTypes.ReachCursor: Projectile projectile = (entity as Item)?.GetComponent(); @@ -161,19 +207,21 @@ namespace Barotrauma return; } - DelayList.Add(new DelayedListElement(this, entity, currentTargets, Vector2.Distance(entity.WorldPosition, user.CursorWorldPosition), worldPosition, entity.WorldPosition)); + var reachCursorElement = new DelayedListElement(this, entity, localTargets, Vector2.Distance(entity.WorldPosition, user.CursorWorldPosition), worldPosition, entity.WorldPosition); + DelayListDict.TryAdd(reachCursorElement.Id, reachCursorElement); break; } } public static void Update(float deltaTime) { - for (int i = DelayList.Count - 1; i >= 0; i--) + // Thread-safe iteration over ConcurrentDictionary + foreach (var kvp in DelayListDict) { - DelayedListElement element = DelayList[i]; + DelayedListElement element = kvp.Value; if (element.Parent.CheckConditionalAlways && !element.Parent.HasRequiredConditions(element.Targets)) { - DelayList.Remove(element); + DelayListDict.TryRemove(element.Id, out _); continue; } @@ -187,7 +235,7 @@ namespace Barotrauma //keep refreshing the position until the effect runs (so e.g. a delayed effect runs at the last known position of a monster before it despawned) if (element.GetPositionBasedOnTargets && element.Entity is { Removed: false }) { - element.WorldPosition = element.Parent.GetPosition(element.Entity, element.Parent.currentTargets); + element.WorldPosition = element.Parent.GetPosition(element.Entity, element.Parent.CurrentTargets); } continue; } @@ -198,8 +246,8 @@ namespace Barotrauma } element.Parent.Apply(deltaTime, element.Entity, element.Targets, element.WorldPosition); - DelayList.Remove(element); + DelayListDict.TryRemove(element.Id, out _); } } } -} \ No newline at end of file +} diff --git a/Barotrauma/BarotraumaShared/SharedSource/StatusEffects/StatusEffect.cs b/Barotrauma/BarotraumaShared/SharedSource/StatusEffects/StatusEffect.cs index a0cd1be2a..7aaf375bc 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/StatusEffects/StatusEffect.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/StatusEffects/StatusEffect.cs @@ -7,15 +7,18 @@ using FarseerPhysics.Dynamics; using Microsoft.Xna.Framework; using Steamworks; using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; +using System.Threading; using System.Xml.Linq; namespace Barotrauma { class DurationListElement { + public readonly long Id; public readonly StatusEffect Parent; public readonly Entity Entity; public float Duration @@ -24,12 +27,24 @@ namespace Barotrauma private set; } public readonly List Targets; - public Character User { get; private set; } + + private volatile Character _user; + public Character User + { + get => _user; + private set => _user = value; + } - public float Timer; + private volatile float _timer; + public float Timer + { + get => _timer; + set => _timer = value; + } public DurationListElement(StatusEffect parentEffect, Entity parentEntity, IEnumerable targets, float duration, Character user) { + Id = Interlocked.Increment(ref StatusEffect._durationElementIdCounter); Parent = parentEffect; Entity = parentEntity; Targets = new List(targets); @@ -39,8 +54,9 @@ namespace Barotrauma public void Reset(float duration, Character newUser) { - Timer = Duration = duration; - User = newUser; + Duration = duration; + Volatile.Write(ref _timer, duration); + _user = newUser; } } @@ -601,14 +617,23 @@ namespace Barotrauma private readonly float lifeTime; private float lifeTimer; - private Dictionary intervalTimers; + private ConcurrentDictionary intervalTimers; /// /// Makes the effect only execute once. After it has executed, it'll never execute again (during the same round). /// private readonly bool oneShot; - public static readonly List DurationList = new List(); + // Thread-safe counter for generating unique IDs for DurationListElement + internal static long _durationElementIdCounter; + + // Thread-safe dictionary for duration effects + public static readonly ConcurrentDictionary DurationListDict = new ConcurrentDictionary(); + + /// + /// Provides a thread-safe enumerable view of the duration list for iteration. + /// + public static IEnumerable DurationList => DurationListDict.Values; /// /// Only applicable for StatusEffects with a duration or delay. Should the conditional checks only be done when the effect triggers, @@ -1624,22 +1649,52 @@ namespace Barotrauma } } - private static readonly List intervalsToRemove = new List(); + // Thread-local list to avoid contention when cleaning up removed entities + [ThreadStatic] + private static List _threadLocalIntervalsToRemove; + + private static List IntervalsToRemove + { + get + { + _threadLocalIntervalsToRemove ??= new List(); + return _threadLocalIntervalsToRemove; + } + } public bool ShouldWaitForInterval(Entity entity, float deltaTime) { - if (Interval > 0.0f && entity != null && intervalTimers != null) + if (Interval > 0.0f && entity != null) { - if (intervalTimers.ContainsKey(entity)) + // Thread-safe lazy initialization + if (intervalTimers == null) { - intervalTimers[entity] -= deltaTime; - if (intervalTimers[entity] > 0.0f) { return true; } + Interlocked.CompareExchange(ref intervalTimers, new ConcurrentDictionary(), null); } - intervalsToRemove.Clear(); - intervalsToRemove.AddRange(intervalTimers.Keys.Where(e => e.Removed)); - foreach (var toRemove in intervalsToRemove) + + if (intervalTimers.TryGetValue(entity, out float currentTimer)) { - intervalTimers.Remove(toRemove); + float newTimer = currentTimer - deltaTime; + if (newTimer > 0.0f) + { + intervalTimers.AddOrUpdate(entity, newTimer, (_, __) => newTimer); + return true; + } + } + + // Clean up removed entities using thread-local list + var toRemove = IntervalsToRemove; + toRemove.Clear(); + foreach (var key in intervalTimers.Keys) + { + if (key.Removed) + { + toRemove.Add(key); + } + } + foreach (var key in toRemove) + { + intervalTimers.TryRemove(key, out _); } } return false; @@ -1655,7 +1710,7 @@ namespace Barotrauma if (Duration > 0.0f && !Stackable) { //ignore if not stackable and there's already an identical statuseffect - DurationListElement existingEffect = DurationList.Find(d => d.Parent == this && d.Targets.FirstOrDefault() == target); + DurationListElement existingEffect = FindExistingDurationEffect(target); if (existingEffect != null) { if (ResetDurationWhenReapplied) @@ -1666,30 +1721,74 @@ namespace Barotrauma } } - currentTargets.Clear(); - currentTargets.Add(target); - if (!HasRequiredConditions(currentTargets)) { return; } - Apply(deltaTime, entity, currentTargets, worldPosition); + var targets = CurrentTargets; + targets.Clear(); + targets.Add(target); + if (!HasRequiredConditions(targets)) { return; } + Apply(deltaTime, entity, targets, worldPosition); } - protected readonly List currentTargets = new List(); + // Thread-local list to avoid contention when collecting targets + [ThreadStatic] + private static List _threadLocalCurrentTargets; + + protected List CurrentTargets + { + get + { + _threadLocalCurrentTargets ??= new List(); + return _threadLocalCurrentTargets; + } + } + + /// + /// Thread-safe method to find an existing duration effect for a single target. + /// + private DurationListElement FindExistingDurationEffect(ISerializableEntity target) + { + foreach (var element in DurationListDict.Values) + { + if (element.Parent == this && element.Targets.FirstOrDefault() == target) + { + return element; + } + } + return null; + } + + /// + /// Thread-safe method to find an existing duration effect for multiple targets. + /// + private DurationListElement FindExistingDurationEffect(IReadOnlyList targets) + { + foreach (var element in DurationListDict.Values) + { + if (element.Parent == this && element.Targets.SequenceEqual(targets)) + { + return element; + } + } + return null; + } + public virtual void Apply(ActionType type, float deltaTime, Entity entity, IReadOnlyList targets, Vector2? worldPosition = null) { if (Disabled) { return; } if (this.type != type) { return; } if (ShouldWaitForInterval(entity, deltaTime)) { return; } - currentTargets.Clear(); + var localTargets = CurrentTargets; + localTargets.Clear(); foreach (ISerializableEntity target in targets) { if (!IsValidTarget(target)) { continue; } - currentTargets.Add(target); + localTargets.Add(target); } - if (TargetIdentifiers != null && currentTargets.Count == 0) { return; } + if (TargetIdentifiers != null && localTargets.Count == 0) { return; } bool hasRequiredItems = HasRequiredItems(entity); - if (!hasRequiredItems || !HasRequiredConditions(currentTargets)) + if (!hasRequiredItems || !HasRequiredConditions(localTargets)) { #if CLIENT if (!hasRequiredItems && playSoundOnRequiredItemFailure) @@ -1703,15 +1802,15 @@ namespace Barotrauma if (Duration > 0.0f && !Stackable) { //ignore if not stackable and there's already an identical statuseffect - DurationListElement existingEffect = DurationList.Find(d => d.Parent == this && d.Targets.SequenceEqual(currentTargets)); + DurationListElement existingEffect = FindExistingDurationEffect(localTargets); if (existingEffect != null) { - existingEffect?.Reset(Math.Max(existingEffect.Timer, Duration), user); + existingEffect.Reset(Math.Max(existingEffect.Timer, Duration), user); return; } } - Apply(deltaTime, entity, currentTargets, worldPosition); + Apply(deltaTime, entity, localTargets, worldPosition); } private Hull GetHull(Entity entity) @@ -1924,7 +2023,8 @@ namespace Barotrauma if (Duration > 0.0f) { - DurationList.Add(new DurationListElement(this, entity, targets, Duration, user)); + var element = new DurationListElement(this, entity, targets, Duration, user); + DurationListDict.TryAdd(element.Id, element); } else { @@ -2452,7 +2552,7 @@ namespace Barotrauma } if (Interval > 0.0f && entity != null) { - intervalTimers ??= new Dictionary(); + intervalTimers ??= new ConcurrentDictionary(); intervalTimers[entity] = Interval; } } @@ -2849,13 +2949,15 @@ namespace Barotrauma UpdateAllProjSpecific(deltaTime); DelayedEffect.Update(deltaTime); - for (int i = DurationList.Count - 1; i >= 0; i--) + + // Thread-safe iteration over ConcurrentDictionary + foreach (var kvp in DurationListDict) { - DurationListElement element = DurationList[i]; + DurationListElement element = kvp.Value; if (element.Parent.CheckConditionalAlways && !element.Parent.HasRequiredConditions(element.Targets)) { - DurationList.RemoveAt(i); + DurationListDict.TryRemove(element.Id, out _); continue; } @@ -2864,7 +2966,7 @@ namespace Barotrauma (t is Limb limb && (limb.character == null || limb.character.Removed))); if (element.Targets.Count == 0) { - DurationList.RemoveAt(i); + DurationListDict.TryRemove(element.Id, out _); continue; } @@ -2959,7 +3061,7 @@ namespace Barotrauma element.Timer -= deltaTime; if (element.Timer > 0.0f) { continue; } - DurationList.Remove(element); + DurationListDict.TryRemove(element.Id, out _); } } @@ -3059,8 +3161,8 @@ namespace Barotrauma public static void StopAll() { CoroutineManager.StopCoroutines("statuseffect"); - DelayedEffect.DelayList.Clear(); - DurationList.Clear(); + DelayedEffect.DelayListDict.Clear(); + DurationListDict.Clear(); } public void AddTag(Identifier tag)