From 59bf2749dd9260d250757494b861dfef75baf036 Mon Sep 17 00:00:00 2001 From: Eero Date: Sun, 28 Dec 2025 16:18:49 +0800 Subject: [PATCH] Improve thread safety in sound and physics systems Refactored SoundChannel and SoundManager to use explicit locking for OpenAL operations and channel assignment, preventing race conditions during parallel sound playback. Added thread-local stacks in DynamicTree to ensure thread safety during parallel physics queries and raycasts. These changes address concurrency issues when sounds or physics queries are triggered from multiple threads. --- .../ClientSource/Sounds/SoundChannel.cs | 152 ++++++++++-------- .../ClientSource/Sounds/SoundManager.cs | 58 +++++-- .../Collision/DynamicTree.cs | 48 +++--- 3 files changed, 154 insertions(+), 104 deletions(-) diff --git a/Barotrauma/BarotraumaClient/ClientSource/Sounds/SoundChannel.cs b/Barotrauma/BarotraumaClient/ClientSource/Sounds/SoundChannel.cs index df9d68998..ade827da5 100644 --- a/Barotrauma/BarotraumaClient/ClientSource/Sounds/SoundChannel.cs +++ b/Barotrauma/BarotraumaClient/ClientSource/Sounds/SoundChannel.cs @@ -503,86 +503,102 @@ namespace Barotrauma.Sounds mutex = new object(); } + // Use the playingChannels lock to protect both channel assignment AND OpenAL operations. + // This prevents race conditions when multiple threads try to play sounds simultaneously + // (e.g., during Parallel.ForEach in MapEntity.UpdateAll). + int poolIndex = (int)sound.SourcePoolIndex; + object channelsLock = sound.Owner.GetPlayingChannelsLock(sound.SourcePoolIndex); + #if !DEBUG try { #endif - if (mutex != null) { Monitor.Enter(mutex); } - if (sound.Owner.CountPlayingInstances(sound) < sound.MaxSimultaneousInstances) + lock (channelsLock) { - ALSourceIndex = sound.Owner.AssignFreeSourceToChannel(this); - } - - if (ALSourceIndex >= 0) - { - if (!IsStream) + if (mutex != null) { Monitor.Enter(mutex); } + try { - Al.Sourcei(sound.Owner.GetSourceFromIndex(Sound.SourcePoolIndex, ALSourceIndex), Al.Buffer, 0); - int alError = Al.GetError(); - if (alError != Al.NoError) + if (sound.Owner.CountPlayingInstancesUnsafe(sound, poolIndex) < sound.MaxSimultaneousInstances) { - throw new Exception("Failed to reset source buffer: " + debugName + ", " + Al.GetErrorString(alError)); + ALSourceIndex = sound.Owner.AssignFreeSourceToChannelUnsafe(this, poolIndex); } - Sound.FillAlBuffers(); - if (Sound.Buffers is not { AlBuffer: not 0, AlMuffledBuffer: not 0 }) { return; } - - uint alBuffer = sound.Owner.GetCategoryMuffle(category) || muffled ? Sound.Buffers.AlMuffledBuffer : Sound.Buffers.AlBuffer; - Al.Sourcei(sound.Owner.GetSourceFromIndex(Sound.SourcePoolIndex, ALSourceIndex), Al.Buffer, (int)alBuffer); - alError = Al.GetError(); - if (alError != Al.NoError) + if (ALSourceIndex >= 0) { - throw new Exception("Failed to bind buffer to source (" + ALSourceIndex.ToString() + ":" + sound.Owner.GetSourceFromIndex(Sound.SourcePoolIndex, ALSourceIndex) + "," + alBuffer.ToString() + "): " + debugName + ", " + Al.GetErrorString(alError)); - } + if (!IsStream) + { + Al.Sourcei(sound.Owner.GetSourceFromIndex(Sound.SourcePoolIndex, ALSourceIndex), Al.Buffer, 0); + int alError = Al.GetError(); + if (alError != Al.NoError) + { + throw new Exception("Failed to reset source buffer: " + debugName + ", " + Al.GetErrorString(alError)); + } - SetProperties(); + Sound.FillAlBuffers(); + if (Sound.Buffers is not { AlBuffer: not 0, AlMuffledBuffer: not 0 }) { return; } - Al.SourcePlay(sound.Owner.GetSourceFromIndex(Sound.SourcePoolIndex, ALSourceIndex)); - alError = Al.GetError(); - if (alError != Al.NoError) - { - throw new Exception("Failed to play source: " + debugName + ", " + Al.GetErrorString(alError)); + uint alBuffer = sound.Owner.GetCategoryMuffle(category) || muffled ? Sound.Buffers.AlMuffledBuffer : Sound.Buffers.AlBuffer; + Al.Sourcei(sound.Owner.GetSourceFromIndex(Sound.SourcePoolIndex, ALSourceIndex), Al.Buffer, (int)alBuffer); + alError = Al.GetError(); + if (alError != Al.NoError) + { + throw new Exception("Failed to bind buffer to source (" + ALSourceIndex.ToString() + ":" + sound.Owner.GetSourceFromIndex(Sound.SourcePoolIndex, ALSourceIndex) + "," + alBuffer.ToString() + "): " + debugName + ", " + Al.GetErrorString(alError)); + } + + SetProperties(); + + Al.SourcePlay(sound.Owner.GetSourceFromIndex(Sound.SourcePoolIndex, ALSourceIndex)); + alError = Al.GetError(); + if (alError != Al.NoError) + { + throw new Exception("Failed to play source: " + debugName + ", " + Al.GetErrorString(alError)); + } + } + else + { + uint alBuffer = 0; + Al.Sourcei(sound.Owner.GetSourceFromIndex(Sound.SourcePoolIndex, ALSourceIndex), Al.Buffer, (int)alBuffer); + int alError = Al.GetError(); + if (alError != Al.NoError) + { + throw new Exception("Failed to reset source buffer: " + debugName + ", " + Al.GetErrorString(alError)); + } + + Al.Sourcei(sound.Owner.GetSourceFromIndex(Sound.SourcePoolIndex, ALSourceIndex), Al.Looping, Al.False); + alError = Al.GetError(); + if (alError != Al.NoError) + { + throw new Exception("Failed to set stream looping state: " + debugName + ", " + Al.GetErrorString(alError)); + } + + streamShortBuffer = new short[STREAM_BUFFER_SIZE]; + + streamBuffers = new uint[4]; + unqueuedBuffers = new uint[4]; + streamBufferAmplitudes = new float[4]; + for (int i = 0; i < 4; i++) + { + Al.GenBuffer(out streamBuffers[i]); + + alError = Al.GetError(); + if (alError != Al.NoError) + { + throw new Exception("Failed to generate stream buffers: " + debugName + ", " + Al.GetErrorString(alError)); + } + + if (!Al.IsBuffer(streamBuffers[i])) + { + throw new Exception("Generated streamBuffer[" + i.ToString() + "] is invalid! " + debugName); + } + } + Sound.Owner.InitUpdateChannelThread(); + SetProperties(); + } } } - else + finally { - uint alBuffer = 0; - Al.Sourcei(sound.Owner.GetSourceFromIndex(Sound.SourcePoolIndex, ALSourceIndex), Al.Buffer, (int)alBuffer); - int alError = Al.GetError(); - if (alError != Al.NoError) - { - throw new Exception("Failed to reset source buffer: " + debugName + ", " + Al.GetErrorString(alError)); - } - - Al.Sourcei(sound.Owner.GetSourceFromIndex(Sound.SourcePoolIndex, ALSourceIndex), Al.Looping, Al.False); - alError = Al.GetError(); - if (alError != Al.NoError) - { - throw new Exception("Failed to set stream looping state: " + debugName + ", " + Al.GetErrorString(alError)); - } - - streamShortBuffer = new short[STREAM_BUFFER_SIZE]; - - streamBuffers = new uint[4]; - unqueuedBuffers = new uint[4]; - streamBufferAmplitudes = new float[4]; - for (int i = 0; i < 4; i++) - { - Al.GenBuffer(out streamBuffers[i]); - - alError = Al.GetError(); - if (alError != Al.NoError) - { - throw new Exception("Failed to generate stream buffers: " + debugName + ", " + Al.GetErrorString(alError)); - } - - if (!Al.IsBuffer(streamBuffers[i])) - { - throw new Exception("Generated streamBuffer[" + i.ToString() + "] is invalid! " + debugName); - } - } - Sound.Owner.InitUpdateChannelThread(); - SetProperties(); + if (mutex != null) { Monitor.Exit(mutex); } } } #if !DEBUG @@ -591,12 +607,6 @@ namespace Barotrauma.Sounds { throw; } - finally - { -#endif - if (mutex != null) { Monitor.Exit(mutex); } -#if !DEBUG - } #endif void SetProperties() diff --git a/Barotrauma/BarotraumaClient/ClientSource/Sounds/SoundManager.cs b/Barotrauma/BarotraumaClient/ClientSource/Sounds/SoundManager.cs index 0826aa2d3..e320bf3d9 100644 --- a/Barotrauma/BarotraumaClient/ClientSource/Sounds/SoundManager.cs +++ b/Barotrauma/BarotraumaClient/ClientSource/Sounds/SoundManager.cs @@ -417,6 +417,15 @@ namespace Barotrauma.Sounds return sourcePools[(int)poolIndex].ALSources[srcInd]; } + /// + /// Gets the lock object for the playing channels array for a specific pool. + /// Used to protect OpenAL operations that need to be atomic with channel assignment. + /// + public object GetPlayingChannelsLock(SourcePoolIndex poolIndex) + { + return playingChannels[(int)poolIndex]; + } + public int AssignFreeSourceToChannel(SoundChannel newChannel) { if (Disabled) { return -1; } @@ -427,14 +436,25 @@ namespace Barotrauma.Sounds lock (playingChannels[poolIndex]) { - for (int i = 0; i < playingChannels[poolIndex].Length; i++) + return AssignFreeSourceToChannelUnsafe(newChannel, poolIndex); + } + } + + /// + /// Assigns a free source to a channel without locking. + /// Caller MUST hold the playingChannels[poolIndex] lock before calling this method. + /// + public int AssignFreeSourceToChannelUnsafe(SoundChannel newChannel, int poolIndex) + { + if (Disabled) { return -1; } + + for (int i = 0; i < playingChannels[poolIndex].Length; i++) + { + if (playingChannels[poolIndex][i] == null || !playingChannels[poolIndex][i].IsPlaying) { - if (playingChannels[poolIndex][i] == null || !playingChannels[poolIndex][i].IsPlaying) - { - if (playingChannels[poolIndex][i] != null) { playingChannels[poolIndex][i].Dispose(); } - playingChannels[poolIndex][i] = newChannel; - return i; - } + if (playingChannels[poolIndex][i] != null) { playingChannels[poolIndex][i].Dispose(); } + playingChannels[poolIndex][i] = newChannel; + return i; } } @@ -476,13 +496,25 @@ namespace Barotrauma.Sounds int count = 0; lock (playingChannels[(int)sound.SourcePoolIndex]) { - for (int i = 0; i < playingChannels[(int)sound.SourcePoolIndex].Length; i++) + count = CountPlayingInstancesUnsafe(sound, (int)sound.SourcePoolIndex); + } + return count; + } + + /// + /// Counts playing instances without locking. + /// Caller MUST hold the playingChannels[poolIndex] lock before calling this method. + /// + public int CountPlayingInstancesUnsafe(Sound sound, int poolIndex) + { + if (Disabled) { return 0; } + int count = 0; + for (int i = 0; i < playingChannels[poolIndex].Length; i++) + { + if (playingChannels[poolIndex][i] != null && + playingChannels[poolIndex][i].Sound.Filename == sound.Filename) { - if (playingChannels[(int)sound.SourcePoolIndex][i] != null && - playingChannels[(int)sound.SourcePoolIndex][i].Sound.Filename == sound.Filename) - { - if (playingChannels[(int)sound.SourcePoolIndex][i].IsPlaying) { count++; }; - } + if (playingChannels[poolIndex][i].IsPlaying) { count++; }; } } return count; diff --git a/Libraries/Farseer Physics Engine 3.5/Collision/DynamicTree.cs b/Libraries/Farseer Physics Engine 3.5/Collision/DynamicTree.cs index fac9982bd..6bc690f09 100644 --- a/Libraries/Farseer Physics Engine 3.5/Collision/DynamicTree.cs +++ b/Libraries/Farseer Physics Engine 3.5/Collision/DynamicTree.cs @@ -30,6 +30,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Threading; using FarseerPhysics.Common; using FarseerPhysics.Dynamics; using Microsoft.Xna.Framework; @@ -74,8 +75,15 @@ namespace FarseerPhysics.Collision /// public class DynamicTree { - private Stack _raycastStack = new Stack(256); - private Stack _queryStack = new Stack(256); + // Thread-local stacks to ensure thread safety during parallel queries/raycasts + [ThreadStatic] + private static Stack _raycastStack; + [ThreadStatic] + private static Stack _queryStack; + + private static Stack RaycastStack => _raycastStack ??= new Stack(256); + private static Stack QueryStack => _queryStack ??= new Stack(256); + private int _freeList; private int _nodeCapacity; private int _nodeCount; @@ -346,12 +354,12 @@ namespace FarseerPhysics.Collision /// The aabb. public void Query(Func callback, ref AABB aabb, ref Body body) { - _queryStack.Clear(); - _queryStack.Push(_root); + QueryStack.Clear(); + QueryStack.Push(_root); - while (_queryStack.Count > 0) + while (QueryStack.Count > 0) { - int nodeId = _queryStack.Pop(); + int nodeId = QueryStack.Pop(); if (nodeId == NullNode) { continue; @@ -386,8 +394,8 @@ namespace FarseerPhysics.Collision } else { - _queryStack.Push(node.Child1); - _queryStack.Push(node.Child2); + QueryStack.Push(node.Child1); + QueryStack.Push(node.Child2); } } } @@ -395,12 +403,12 @@ namespace FarseerPhysics.Collision public void Query(Func callback, ref AABB aabb) { - _queryStack.Clear(); - _queryStack.Push(_root); + QueryStack.Clear(); + QueryStack.Push(_root); - while (_queryStack.Count > 0) + while (QueryStack.Count > 0) { - int nodeId = _queryStack.Pop(); + int nodeId = QueryStack.Pop(); if (nodeId == NullNode) { continue; @@ -419,8 +427,8 @@ namespace FarseerPhysics.Collision } else { - _queryStack.Push(_nodes[nodeId].Child1); - _queryStack.Push(_nodes[nodeId].Child2); + QueryStack.Push(_nodes[nodeId].Child1); + QueryStack.Push(_nodes[nodeId].Child2); } } } @@ -460,12 +468,12 @@ namespace FarseerPhysics.Collision Vector2.Max(ref p1, ref t, out segmentAABB.UpperBound); } - _raycastStack.Clear(); - _raycastStack.Push(_root); + RaycastStack.Clear(); + RaycastStack.Push(_root); - while (_raycastStack.Count > 0) + while (RaycastStack.Count > 0) { - int nodeId = _raycastStack.Pop(); + int nodeId = RaycastStack.Pop(); if (nodeId == NullNode) { continue; @@ -522,8 +530,8 @@ namespace FarseerPhysics.Collision } else { - _raycastStack.Push(_nodes[nodeId].Child1); - _raycastStack.Push(_nodes[nodeId].Child2); + RaycastStack.Push(_nodes[nodeId].Child1); + RaycastStack.Push(_nodes[nodeId].Child2); } } }