From 45312af297678ce4500c682ef13a7459e4f56139 Mon Sep 17 00:00:00 2001 From: Eero Date: Sun, 28 Dec 2025 14:14:53 +0800 Subject: [PATCH] WIP Make static collections thread-safe using ThreadStatic and ThreadLocal Refactored various static and instance collections to use [ThreadStatic], ThreadLocal, or local variables to prevent concurrent modification issues during parallel updates. This affects status effect targets, affliction lists, damage modifiers, and cached data in Character, CharacterHealth, Limb, Explosion, Hull, Submarine, and ToolBox classes. Also replaced Dictionary caches with ConcurrentDictionary where appropriate for thread safety. --- .../SharedSource/Characters/Character.cs | 7 +- .../Characters/Health/CharacterHealth.cs | 59 ++++++++++++---- .../SharedSource/Characters/Limb.cs | 24 ++++--- .../SharedSource/Map/Explosion.cs | 7 +- .../BarotraumaShared/SharedSource/Map/Hull.cs | 14 ++-- .../SharedSource/Map/Submarine.cs | 70 +++++++++++-------- .../SharedSource/Utils/ToolBox.cs | 29 ++++---- 7 files changed, 135 insertions(+), 75 deletions(-) diff --git a/Barotrauma/BarotraumaShared/SharedSource/Characters/Character.cs b/Barotrauma/BarotraumaShared/SharedSource/Characters/Character.cs index b0e7aff11..2ab6d6a68 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Characters/Character.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Characters/Character.cs @@ -4880,7 +4880,11 @@ namespace Barotrauma HealthUpdateInterval = 0.0f; } - private readonly List targets = new List(); + // Thread-static to avoid concurrent modification in parallel item updates + [ThreadStatic] + private static List t_statusEffectTargets; + private static List StatusEffectTargets => t_statusEffectTargets ??= new List(); + public void ApplyStatusEffects(ActionType actionType, float deltaTime) { if (actionType == ActionType.OnEating) @@ -4909,6 +4913,7 @@ namespace Barotrauma if (statusEffect.HasTargetType(StatusEffect.TargetType.NearbyItems) || statusEffect.HasTargetType(StatusEffect.TargetType.NearbyCharacters)) { + var targets = StatusEffectTargets; targets.Clear(); statusEffect.AddNearbyTargets(WorldPosition, targets); statusEffect.Apply(actionType, deltaTime, this, targets); diff --git a/Barotrauma/BarotraumaShared/SharedSource/Characters/Health/CharacterHealth.cs b/Barotrauma/BarotraumaShared/SharedSource/Characters/Health/CharacterHealth.cs index a4c538451..03fecbc34 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Characters/Health/CharacterHealth.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Characters/Health/CharacterHealth.cs @@ -537,20 +537,25 @@ namespace Barotrauma return false; } - private readonly List matchingAfflictions = new List(); + // Thread-static to avoid concurrent modification in parallel item updates + [ThreadStatic] + private static List t_matchingAfflictions; + private static List MatchingAfflictions => t_matchingAfflictions ??= new List(); public void ReduceAllAfflictionsOnAllLimbs(float amount, ActionType? treatmentAction = null) { + var matchingAfflictions = MatchingAfflictions; matchingAfflictions.Clear(); matchingAfflictions.AddRange(afflictions.Keys); - ReduceMatchingAfflictions(amount, treatmentAction); + ReduceMatchingAfflictions(matchingAfflictions, amount, treatmentAction); } public void ReduceAfflictionOnAllLimbs(Identifier afflictionIdOrType, float amount, ActionType? treatmentAction = null, Character attacker = null) { if (afflictionIdOrType.IsEmpty) { throw new ArgumentException($"{nameof(afflictionIdOrType)} is empty"); } + var matchingAfflictions = MatchingAfflictions; matchingAfflictions.Clear(); foreach (var affliction in afflictions) { @@ -560,7 +565,7 @@ namespace Barotrauma } } - ReduceMatchingAfflictions(amount, treatmentAction, attacker); + ReduceMatchingAfflictions(matchingAfflictions, amount, treatmentAction, attacker); } private IEnumerable GetAfflictionsForLimb(Limb targetLimb) @@ -570,10 +575,11 @@ namespace Barotrauma { if (targetLimb is null) { throw new ArgumentNullException(nameof(targetLimb)); } + var matchingAfflictions = MatchingAfflictions; matchingAfflictions.Clear(); matchingAfflictions.AddRange(GetAfflictionsForLimb(targetLimb)); - ReduceMatchingAfflictions(amount, treatmentAction); + ReduceMatchingAfflictions(matchingAfflictions, amount, treatmentAction); } public void ReduceAfflictionOnLimb(Limb targetLimb, Identifier afflictionIdOrType, float amount, ActionType? treatmentAction = null, Character attacker = null) @@ -581,6 +587,7 @@ namespace Barotrauma if (afflictionIdOrType.IsEmpty) { throw new ArgumentException($"{nameof(afflictionIdOrType)} is empty"); } if (targetLimb is null) { throw new ArgumentNullException(nameof(targetLimb)); } + var matchingAfflictions = MatchingAfflictions; matchingAfflictions.Clear(); var targetLimbHealth = limbHealths[targetLimb.HealthIndex]; foreach (var affliction in afflictions) @@ -591,10 +598,10 @@ namespace Barotrauma matchingAfflictions.Add(affliction.Key); } } - ReduceMatchingAfflictions(amount, treatmentAction, attacker); + ReduceMatchingAfflictions(matchingAfflictions, amount, treatmentAction, attacker); } - private void ReduceMatchingAfflictions(float amount, ActionType? treatmentAction, Character attacker = null) + private void ReduceMatchingAfflictions(List matchingAfflictions, float amount, ActionType? treatmentAction, Character attacker = null) { if (matchingAfflictions.Count == 0) { return; } @@ -681,12 +688,19 @@ namespace Barotrauma } } - private readonly static List afflictionsToRemove = new List(); - private readonly static List> afflictionsToUpdate = new List>(); + // Thread-static to avoid concurrent modification when multiple characters are updated in parallel + [ThreadStatic] + private static List t_afflictionsToRemove; + [ThreadStatic] + private static List> t_afflictionsToUpdate; + private static List AfflictionsToRemove => t_afflictionsToRemove ??= new List(); + private static List> AfflictionsToUpdate => t_afflictionsToUpdate ??= new List>(); + public void SetAllDamage(float damageAmount, float bleedingDamageAmount, float burnDamageAmount) { if (Unkillable || Character.GodMode) { return; } + var afflictionsToRemove = AfflictionsToRemove; afflictionsToRemove.Clear(); afflictionsToRemove.AddRange(afflictions.Keys.Where(a => a.Prefab.AfflictionType == AfflictionPrefab.InternalDamage.AfflictionType || @@ -737,6 +751,7 @@ namespace Barotrauma public void RemoveAfflictions(Func predicate) { + var afflictionsToRemove = AfflictionsToRemove; afflictionsToRemove.Clear(); afflictionsToRemove.AddRange(afflictions.Keys.Where(affliction => predicate(affliction))); foreach (var affliction in afflictionsToRemove) @@ -748,6 +763,7 @@ namespace Barotrauma public void RemoveAllAfflictions() { + var afflictionsToRemove = AfflictionsToRemove; afflictionsToRemove.Clear(); afflictionsToRemove.AddRange(afflictions.Keys.Where(a => !irremovableAfflictions.ContainsKey(a))); foreach (var affliction in afflictionsToRemove) @@ -765,6 +781,7 @@ namespace Barotrauma public void RemoveNegativeAfflictions() { + var afflictionsToRemove = AfflictionsToRemove; afflictionsToRemove.Clear(); afflictionsToRemove.AddRange(afflictions.Keys.Where(a => !irremovableAfflictions.ContainsKey(a) && @@ -904,6 +921,8 @@ namespace Barotrauma if (!Character.GodMode) { + var afflictionsToRemove = AfflictionsToRemove; + var afflictionsToUpdate = AfflictionsToUpdate; afflictionsToRemove.Clear(); afflictionsToUpdate.Clear(); foreach (KeyValuePair kvp in afflictions) @@ -1198,9 +1217,14 @@ namespace Barotrauma return (causeOfDeath, strongestAffliction); } - private readonly List allAfflictions = new List(); + // Thread-static to avoid concurrent modification in parallel item updates + [ThreadStatic] + private static List t_allAfflictions; + private static List AllAfflictionsList => t_allAfflictions ??= new List(); + private IEnumerable GetAllAfflictions(bool mergeSameAfflictions, Func predicate = null) { + var allAfflictions = AllAfflictionsList; allAfflictions.Clear(); if (!mergeSameAfflictions) { @@ -1383,10 +1407,17 @@ namespace Barotrauma return MathHelper.Clamp(strength, 0.0f, affliction.Prefab.MaxStrength); } - private readonly List activeAfflictions = new List(); - private readonly List<(LimbHealth limbHealth, Affliction affliction)> limbAfflictions = new List<(LimbHealth limbHealth, Affliction affliction)>(); + // Thread-static to avoid concurrent modification in parallel updates + [ThreadStatic] + private static List t_activeAfflictions; + [ThreadStatic] + private static List<(LimbHealth limbHealth, Affliction affliction)> t_limbAfflictions; + private static List ActiveAfflictionsList => t_activeAfflictions ??= new List(); + private static List<(LimbHealth limbHealth, Affliction affliction)> LimbAfflictionsList => t_limbAfflictions ??= new List<(LimbHealth limbHealth, Affliction affliction)>(); + public void ServerWrite(IWriteMessage msg) { + var activeAfflictions = ActiveAfflictionsList; activeAfflictions.Clear(); foreach (KeyValuePair kvp in afflictions) { @@ -1412,6 +1443,7 @@ namespace Barotrauma } } + var limbAfflictions = LimbAfflictionsList; limbAfflictions.Clear(); foreach (KeyValuePair kvp in afflictions) { @@ -1441,8 +1473,9 @@ namespace Barotrauma public void Remove() { RemoveProjSpecific(); - afflictionsToRemove.Clear(); - afflictionsToUpdate.Clear(); + // Clear thread-static lists to help with garbage collection + AfflictionsToRemove.Clear(); + AfflictionsToUpdate.Clear(); } partial void RemoveProjSpecific(); diff --git a/Barotrauma/BarotraumaShared/SharedSource/Characters/Limb.cs b/Barotrauma/BarotraumaShared/SharedSource/Characters/Limb.cs index d9226ce75..79c09a34c 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Characters/Limb.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Characters/Limb.cs @@ -797,16 +797,14 @@ namespace Barotrauma return AddDamage(simPosition, afflictions, playSound); } - private readonly List appliedDamageModifiers = new List(); - private readonly List tempModifiers = new List(); - private readonly List afflictionsCopy = new List(); + // Thread-safe: using local variables instead of instance fields to avoid concurrent modification public AttackResult AddDamage(Vector2 simPosition, IEnumerable afflictions, bool playSound, float damageMultiplier = 1, float penetration = 0f, Character attacker = null) { - appliedDamageModifiers.Clear(); - afflictionsCopy.Clear(); + var appliedDamageModifiers = new List(); + var afflictionsCopy = new List(); foreach (var affliction in afflictions) { - tempModifiers.Clear(); + var tempModifiers = new List(); var newAffliction = affliction; float random = Rand.Value(Rand.RandSync.Unsynced); bool foundMatchingModifier = false; @@ -1022,13 +1020,18 @@ namespace Barotrauma partial void UpdateProjSpecific(float deltaTime); - private readonly List contactBodies = new List(); + // Thread-static to avoid concurrent modification in parallel item updates + [ThreadStatic] + private static List t_contactBodies; + private static List ContactBodies => t_contactBodies ??= new List(); + /// /// Returns true if the attack successfully hit something. If the distance is not given, it will be calculated. /// public bool UpdateAttack(float deltaTime, Vector2 attackSimPos, IDamageable damageTarget, out AttackResult attackResult, float distance = -1, Limb targetLimb = null) { attackResult = default; + var contactBodies = ContactBodies; Vector2 simPos = ragdoll.SimplePhysicsEnabled ? character.SimPosition : SimPosition; float dist = distance > -1 ? distance : ConvertUnits.ToDisplayUnits(Vector2.Distance(simPos, attackSimPos)); bool wasRunning = attack.IsRunning; @@ -1287,7 +1290,11 @@ namespace Barotrauma } } - private readonly List targets = new List(); + // Thread-static to avoid concurrent modification in parallel item updates + [ThreadStatic] + private static List t_statusEffectTargets; + private static List StatusEffectTargets => t_statusEffectTargets ??= new List(); + public void ApplyStatusEffects(ActionType actionType, float deltaTime) { if (!statusEffects.TryGetValue(actionType, out var statusEffectList)) { return; } @@ -1310,6 +1317,7 @@ namespace Barotrauma if (statusEffect.HasTargetType(StatusEffect.TargetType.NearbyItems) || statusEffect.HasTargetType(StatusEffect.TargetType.NearbyCharacters)) { + var targets = StatusEffectTargets; targets.Clear(); statusEffect.AddNearbyTargets(WorldPosition, targets); statusEffect.Apply(actionType, deltaTime, character, targets); diff --git a/Barotrauma/BarotraumaShared/SharedSource/Map/Explosion.cs b/Barotrauma/BarotraumaShared/SharedSource/Map/Explosion.cs index 5c11eebb7..f5c63b773 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Map/Explosion.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Map/Explosion.cs @@ -7,6 +7,7 @@ using Microsoft.Xna.Framework; using System; using System.Collections.Generic; using System.Linq; +using System.Threading; namespace Barotrauma { @@ -648,7 +649,11 @@ namespace Barotrauma } } - private static readonly Dictionary damagedStructures = new Dictionary(); + // ThreadLocal for thread-safe structure damage tracking + private static readonly ThreadLocal> damagedStructuresLocal = + new ThreadLocal>(() => new Dictionary()); + private static Dictionary damagedStructures => damagedStructuresLocal.Value; + /// /// Returns a dictionary where the keys are the structures that took damage and the values are the amount of damage taken /// diff --git a/Barotrauma/BarotraumaShared/SharedSource/Map/Hull.cs b/Barotrauma/BarotraumaShared/SharedSource/Map/Hull.cs index 1c193d983..1d4b733ba 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Map/Hull.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Map/Hull.cs @@ -6,6 +6,7 @@ using System; using System.Collections.Generic; using System.Globalization; using System.Linq; +using System.Threading; using System.Xml.Linq; using Barotrauma.MapCreatures.Behavior; using Barotrauma.Items.Components; @@ -1133,13 +1134,18 @@ namespace Barotrauma } /// - /// Used in + /// Used in - ThreadLocal for thread safety /// - private static readonly Dictionary cachedDistances = []; + private static readonly ThreadLocal> cachedDistancesLocal = + new ThreadLocal>(() => new Dictionary()); /// - /// Used in + /// Used in - ThreadLocal for thread safety /// - private static readonly PriorityQueue<(Hull hull, Vector2 pos), float> priorityQueue = new PriorityQueue<(Hull hull, Vector2 pos), float>(); + private static readonly ThreadLocal> priorityQueueLocal = + new ThreadLocal>(() => new PriorityQueue<(Hull hull, Vector2 pos), float>()); + + private static Dictionary cachedDistances => cachedDistancesLocal.Value; + private static PriorityQueue<(Hull hull, Vector2 pos), float> priorityQueue => priorityQueueLocal.Value; /// /// Approximate distance from this hull to the target hull, moving through open gaps without passing through walls. diff --git a/Barotrauma/BarotraumaShared/SharedSource/Map/Submarine.cs b/Barotrauma/BarotraumaShared/SharedSource/Map/Submarine.cs index dfe6eceb1..4a8e2a3c9 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Map/Submarine.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Map/Submarine.cs @@ -12,6 +12,7 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; using System.Linq; +using System.Threading; using System.Xml.Linq; using Voronoi2; @@ -97,10 +98,11 @@ namespace Barotrauma } } - private static Vector2 lastPickedPosition; - private static float lastPickedFraction; - private static Fixture lastPickedFixture; - private static Vector2 lastPickedNormal; + // ThreadLocal for thread-safe ray casting results + private static readonly ThreadLocal lastPickedPositionLocal = new ThreadLocal(); + private static readonly ThreadLocal lastPickedFractionLocal = new ThreadLocal(); + private static readonly ThreadLocal lastPickedFixtureLocal = new ThreadLocal(); + private static readonly ThreadLocal lastPickedNormalLocal = new ThreadLocal(); private Vector2 prevPosition; @@ -114,22 +116,22 @@ namespace Barotrauma public static Vector2 LastPickedPosition { - get { return lastPickedPosition; } + get { return lastPickedPositionLocal.Value; } } public static float LastPickedFraction { - get { return lastPickedFraction; } + get { return lastPickedFractionLocal.Value; } } public static Fixture LastPickedFixture { - get { return lastPickedFixture; } + get { return lastPickedFixtureLocal.Value; } } public static Vector2 LastPickedNormal { - get { return lastPickedNormal; } + get { return lastPickedNormalLocal.Value; } } public bool Loading @@ -854,10 +856,10 @@ namespace Barotrauma }, ref aabb); if (closestFraction <= 0.0f) { - lastPickedPosition = rayStart; - lastPickedFraction = closestFraction; - lastPickedFixture = closestFixture; - lastPickedNormal = closestNormal; + lastPickedPositionLocal.Value = rayStart; + lastPickedFractionLocal.Value = closestFraction; + lastPickedFixtureLocal.Value = closestFixture; + lastPickedNormalLocal.Value = closestNormal; return closestBody; } } @@ -876,16 +878,22 @@ namespace Barotrauma return fraction; }, rayStart, rayEnd, collisionCategory ?? Category.All); - lastPickedPosition = rayStart + (rayEnd - rayStart) * closestFraction; - lastPickedFraction = closestFraction; - lastPickedFixture = closestFixture; - lastPickedNormal = closestNormal; + lastPickedPositionLocal.Value = rayStart + (rayEnd - rayStart) * closestFraction; + lastPickedFractionLocal.Value = closestFraction; + lastPickedFixtureLocal.Value = closestFixture; + lastPickedNormalLocal.Value = closestNormal; return closestBody; } - private static readonly Dictionary bodyDist = new Dictionary(); - private static readonly List bodies = new List(); + // ThreadLocal for thread-safe body picking + private static readonly ThreadLocal> bodyDistLocal = + new ThreadLocal>(() => new Dictionary()); + private static readonly ThreadLocal> bodiesLocal = + new ThreadLocal>(() => new List()); + + private static Dictionary bodyDist => bodyDistLocal.Value; + private static List bodies => bodiesLocal.Value; public static float LastPickedBodyDist(Body body) { @@ -919,10 +927,10 @@ namespace Barotrauma } if (fraction < closestFraction) { - lastPickedPosition = rayStart + (rayEnd - rayStart) * fraction; - lastPickedFraction = fraction; - lastPickedNormal = normal; - lastPickedFixture = fixture; + lastPickedPositionLocal.Value = rayStart + (rayEnd - rayStart) * fraction; + lastPickedFractionLocal.Value = fraction; + lastPickedNormalLocal.Value = normal; + lastPickedFixtureLocal.Value = fixture; } //continue return -1; @@ -940,10 +948,10 @@ namespace Barotrauma if (!fixture.Shape.TestPoint(ref transform, ref rayStart)) { return true; } closestFraction = 0.0f; - lastPickedPosition = rayStart; - lastPickedFraction = 0.0f; - lastPickedNormal = Vector2.Normalize(rayEnd - rayStart); - lastPickedFixture = fixture; + lastPickedPositionLocal.Value = rayStart; + lastPickedFractionLocal.Value = 0.0f; + lastPickedNormalLocal.Value = Vector2.Normalize(rayEnd - rayStart); + lastPickedFixtureLocal.Value = fixture; bodies.Add(fixture.Body); bodyDist[fixture.Body] = 0.0f; return false; @@ -1011,7 +1019,7 @@ namespace Barotrauma if (Vector2.DistanceSquared(rayStart, rayEnd) < 0.01f) { - lastPickedPosition = rayEnd; + lastPickedPositionLocal.Value = rayEnd; return null; } @@ -1053,10 +1061,10 @@ namespace Barotrauma , rayStart, rayEnd); - lastPickedPosition = rayStart + (rayEnd - rayStart) * closestFraction; - lastPickedFraction = closestFraction; - lastPickedFixture = closestFixture; - lastPickedNormal = closestNormal; + lastPickedPositionLocal.Value = rayStart + (rayEnd - rayStart) * closestFraction; + lastPickedFractionLocal.Value = closestFraction; + lastPickedFixtureLocal.Value = closestFixture; + lastPickedNormalLocal.Value = closestNormal; return closestBody; } diff --git a/Barotrauma/BarotraumaShared/SharedSource/Utils/ToolBox.cs b/Barotrauma/BarotraumaShared/SharedSource/Utils/ToolBox.cs index a96376047..2ab344c36 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/Utils/ToolBox.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/Utils/ToolBox.cs @@ -2,6 +2,7 @@ using Barotrauma.Networking; using Microsoft.Xna.Framework; using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics.CodeAnalysis; @@ -75,7 +76,7 @@ namespace Barotrauma return !corrected; } - private static readonly Dictionary cachedFileNames = new Dictionary(); + private static readonly ConcurrentDictionary cachedFileNames = new ConcurrentDictionary(); public static string CorrectFilenameCase(string filename, out bool corrected, string directory = "") { @@ -153,7 +154,7 @@ namespace Barotrauma if (i < subDirs.Length - 1) { filename += "/"; } } - cachedFileNames.Add(originalFilename, filename); + cachedFileNames.TryAdd(originalFilename, filename); return filename; } @@ -355,32 +356,26 @@ namespace Barotrauma return text; } - private static Dictionary> cachedLines = new Dictionary>(); + private static readonly ConcurrentDictionary> cachedLines = new ConcurrentDictionary>(); public static string GetRandomLine(string filePath, Rand.RandSync randSync = Rand.RandSync.ServerAndClient) { - List lines; - if (cachedLines.ContainsKey(filePath)) - { - lines = cachedLines[filePath]; - } - else + List lines = cachedLines.GetOrAdd(filePath, path => { try { - lines = File.ReadAllLines(filePath, catchUnauthorizedAccessExceptions: false).ToList(); - cachedLines.Add(filePath, lines); - if (lines.Count == 0) + var fileLines = File.ReadAllLines(path, catchUnauthorizedAccessExceptions: false).ToList(); + if (fileLines.Count == 0) { - DebugConsole.ThrowError("File \"" + filePath + "\" is empty!"); - return ""; + DebugConsole.ThrowError("File \"" + path + "\" is empty!"); } + return fileLines; } catch (Exception e) { - DebugConsole.ThrowError("Couldn't open file \"" + filePath + "\"!", e); - return ""; + DebugConsole.ThrowError("Couldn't open file \"" + path + "\"!", e); + return new List(); } - } + }); if (lines.Count == 0) return ""; return lines[Rand.Range(0, lines.Count, randSync)];