From 5a08a9bb43a9f5704a63a729ab2e503921e43782 Mon Sep 17 00:00:00 2001
From: CasualcoderDev <262955786+CasualcoderDev@users.noreply.github.com>
Date: Thu, 23 Jul 2026 19:41:51 +0700
Subject: [PATCH] fix: VulkanHostBufferPool deadlock, audio overflow crash, and
 log grouping (#564)

* fix: VulkanHostBufferPool deadlock, audio overflow crash, and log grouping

* fix: implement thread-safe buffer pool, refactor output handling, and use unchecked cast for audio conversion
---
 src/SharpEmu.GUI/EmulatorProcess.cs           |   2 +-
 src/SharpEmu.Libs/Audio/AudioPcmConversion.cs |   2 +-
 .../VideoOut/VulkanHostBufferPool.cs          | 106 ++++++++++++------
 3 files changed, 71 insertions(+), 39 deletions(-)

diff --git a/src/SharpEmu.GUI/EmulatorProcess.cs b/src/SharpEmu.GUI/EmulatorProcess.cs
index 290d3f9e..822b181c 100644
--- a/src/SharpEmu.GUI/EmulatorProcess.cs
+++ b/src/SharpEmu.GUI/EmulatorProcess.cs
@@ -387,7 +387,7 @@ internal sealed class EmulatorProcess : IDisposable
 
     private void ForwardOutput(string? line, bool isError)
     {
-        if (!string.IsNullOrEmpty(line))
+        if (line is not null)
         {
             OutputReceived?.Invoke(line, isError);
         }
diff --git a/src/SharpEmu.Libs/Audio/AudioPcmConversion.cs b/src/SharpEmu.Libs/Audio/AudioPcmConversion.cs
index 6294402f..0cc2a697 100644
--- a/src/SharpEmu.Libs/Audio/AudioPcmConversion.cs
+++ b/src/SharpEmu.Libs/Audio/AudioPcmConversion.cs
@@ -68,7 +68,7 @@ internal static class AudioPcmConversion
 
         value = Math.Clamp(value, -1.0f, 1.0f);
         var scale = value < 0.0f ? 32768.0f : short.MaxValue;
-        return checked((short)MathF.Round(value * scale));
+        return unchecked((short)MathF.Round(value * scale));
     }
 
     // <paramref name="volume"/> is expected pre-clamped to [0, 1] by the caller.
diff --git a/src/SharpEmu.Libs/VideoOut/VulkanHostBufferPool.cs b/src/SharpEmu.Libs/VideoOut/VulkanHostBufferPool.cs
index b6d2404f..d71d10c3 100644
--- a/src/SharpEmu.Libs/VideoOut/VulkanHostBufferPool.cs
+++ b/src/SharpEmu.Libs/VideoOut/VulkanHostBufferPool.cs
@@ -18,6 +18,7 @@ internal readonly record struct VulkanHostBufferAllocation(
 
 internal sealed class VulkanHostBufferPool : IDisposable
 {
+    private readonly object _gate = new();
     private readonly Dictionary<VulkanHostBufferPoolKey, Stack<VulkanHostBufferAllocation>>
         _available = [];
     private readonly Dictionary<ulong, VulkanHostBufferAllocation> _allocations = [];
@@ -40,16 +41,19 @@ internal sealed class VulkanHostBufferPool : IDisposable
         VulkanHostBufferPoolKey key,
         out VulkanHostBufferAllocation allocation)
     {
-        if (!_available.TryGetValue(key, out var available) ||
-            !available.TryPop(out allocation))
+        lock (_gate)
         {
-            allocation = default;
-            return false;
-        }
+            if (!_available.TryGetValue(key, out var available) ||
+                !available.TryPop(out allocation))
+            {
+                allocation = default;
+                return false;
+            }
 
-        _cachedHandles.Remove(allocation.Buffer.Handle);
-        CachedBytes -= allocation.Key.Capacity;
-        return true;
+            _cachedHandles.Remove(allocation.Buffer.Handle);
+            CachedBytes -= allocation.Key.Capacity;
+            return true;
+        }
     }
 
     public void Register(VulkanHostBufferAllocation allocation)
@@ -59,51 +63,79 @@ internal sealed class VulkanHostBufferPool : IDisposable
             throw new ArgumentException("A pooled buffer must have a valid handle.", nameof(allocation));
         }
 
-        _allocations.Add(allocation.Buffer.Handle, allocation);
+        lock (_gate)
+        {
+            _allocations.Add(allocation.Buffer.Handle, allocation);
+        }
     }
 
     public bool Return(VkBuffer buffer, DeviceMemory memory)
     {
-        if (!_allocations.TryGetValue(buffer.Handle, out var allocation) ||
-            allocation.Memory.Handle != memory.Handle)
+        VulkanHostBufferAllocation? toDestroy = null;
+        lock (_gate)
         {
-            return false;
+            if (!_allocations.TryGetValue(buffer.Handle, out var allocation) ||
+                allocation.Memory.Handle != memory.Handle)
+            {
+                return false;
+            }
+
+            if (!_cachedHandles.Add(buffer.Handle))
+            {
+                return true;
+            }
+
+            if (allocation.Key.Capacity > MaximumCachedBytes - CachedBytes)
+            {
+                _cachedHandles.Remove(buffer.Handle);
+                _allocations.Remove(buffer.Handle);
+                toDestroy = allocation;
+            }
+            else
+            {
+                if (!_available.TryGetValue(allocation.Key, out var available))
+                {
+                    available = [];
+                    _available.Add(allocation.Key, available);
+                }
+
+                available.Push(allocation);
+                CachedBytes += allocation.Key.Capacity;
+            }
         }
 
-        if (!_cachedHandles.Add(buffer.Handle))
-        {
-            return true;
+        // Destroy outside the lock — _destroy calls into Vulkan which may
+        // grab device-level locks, and holding _gate while doing so risks
+        // a lock-ordering deadlock with a thread that holds the device lock
+        // and is waiting on _gate.
+if (toDestroy is { } td)
+{
+    _destroy(td);
+
         }
 
-        if (allocation.Key.Capacity > MaximumCachedBytes - CachedBytes)
-        {
-            _cachedHandles.Remove(buffer.Handle);
-            _allocations.Remove(buffer.Handle);
-            _destroy(allocation);
-            return true;
-        }
-
-        if (!_available.TryGetValue(allocation.Key, out var available))
-        {
-            available = [];
-            _available.Add(allocation.Key, available);
-        }
-
-        available.Push(allocation);
-        CachedBytes += allocation.Key.Capacity;
         return true;
     }
 
     public void Dispose()
     {
-        foreach (var allocation in _allocations.Values)
+        // Snapshot under the lock, destroy outside — _destroy calls into
+        // Vulkan which may grab device-level locks; holding _gate while
+        // doing so risks a lock-ordering deadlock with any thread that
+        // acquires the device lock first and then waits on _gate.
+        List<VulkanHostBufferAllocation> toDestroy;
+        lock (_gate)
+        {
+            toDestroy = new List<VulkanHostBufferAllocation>(_allocations.Values);
+            _allocations.Clear();
+            _available.Clear();
+            _cachedHandles.Clear();
+            CachedBytes = 0;
+        }
+
+        foreach (var allocation in toDestroy)
         {
             _destroy(allocation);
         }
-
-        _allocations.Clear();
-        _available.Clear();
-        _cachedHandles.Clear();
-        CachedBytes = 0;
     }
 }