From 4201079b349c34372aa9375791aa86d7e90572f1 Mon Sep 17 00:00:00 2001
From: Claus Vium <cvium@users.noreply.github.com>
Date: Sat, 30 Mar 2024 17:30:00 +0100
Subject: [PATCH] fix: use a reentrant lock when accessing active connections
 (#11256)

---
 .../HttpServer/WebSocketManager.cs            | 22 ++++++-----
 .../Net/BasePeriodicWebSocketListener.cs      | 37 +++----------------
 2 files changed, 18 insertions(+), 41 deletions(-)

diff --git a/Emby.Server.Implementations/HttpServer/WebSocketManager.cs b/Emby.Server.Implementations/HttpServer/WebSocketManager.cs
index 52f14b0b10..774d3563cb 100644
--- a/Emby.Server.Implementations/HttpServer/WebSocketManager.cs
+++ b/Emby.Server.Implementations/HttpServer/WebSocketManager.cs
@@ -48,7 +48,7 @@ namespace Emby.Server.Implementations.HttpServer
 
                 WebSocket webSocket = await context.WebSockets.AcceptWebSocketAsync().ConfigureAwait(false);
 
-                using var connection = new WebSocketConnection(
+                var connection = new WebSocketConnection(
                     _loggerFactory.CreateLogger<WebSocketConnection>(),
                     webSocket,
                     authorizationInfo,
@@ -56,17 +56,19 @@ namespace Emby.Server.Implementations.HttpServer
                 {
                     OnReceive = ProcessWebSocketMessageReceived
                 };
-
-                var tasks = new Task[_webSocketListeners.Length];
-                for (var i = 0; i < _webSocketListeners.Length; ++i)
+                await using (connection.ConfigureAwait(false))
                 {
-                    tasks[i] = _webSocketListeners[i].ProcessWebSocketConnectedAsync(connection, context);
+                    var tasks = new Task[_webSocketListeners.Length];
+                    for (var i = 0; i < _webSocketListeners.Length; ++i)
+                    {
+                        tasks[i] = _webSocketListeners[i].ProcessWebSocketConnectedAsync(connection, context);
+                    }
+
+                    await Task.WhenAll(tasks).ConfigureAwait(false);
+
+                    await connection.ReceiveAsync().ConfigureAwait(false);
+                    _logger.LogInformation("WS {IP} closed", context.Connection.RemoteIpAddress);
                 }
-
-                await Task.WhenAll(tasks).ConfigureAwait(false);
-
-                await connection.ReceiveAsync().ConfigureAwait(false);
-                _logger.LogInformation("WS {IP} closed", context.Connection.RemoteIpAddress);
             }
             catch (Exception ex) // Otherwise ASP.Net will ignore the exception
             {
diff --git a/MediaBrowser.Controller/Net/BasePeriodicWebSocketListener.cs b/MediaBrowser.Controller/Net/BasePeriodicWebSocketListener.cs
index 219da309e4..06386f2b86 100644
--- a/MediaBrowser.Controller/Net/BasePeriodicWebSocketListener.cs
+++ b/MediaBrowser.Controller/Net/BasePeriodicWebSocketListener.cs
@@ -33,7 +33,7 @@ namespace MediaBrowser.Controller.Net
             SingleWriter = false
         });
 
-        private readonly SemaphoreSlim _lock = new(1, 1);
+        private readonly object _activeConnectionsLock = new();
 
         /// <summary>
         /// The _active connections.
@@ -126,15 +126,10 @@ namespace MediaBrowser.Controller.Net
                 InitialDelayMs = dueTimeMs
             };
 
-            _lock.Wait();
-            try
+            lock (_activeConnectionsLock)
             {
                 _activeConnections.Add((message.Connection, cancellationTokenSource, state));
             }
-            finally
-            {
-                _lock.Release();
-            }
         }
 
         protected void SendData(bool force)
@@ -153,8 +148,7 @@ namespace MediaBrowser.Controller.Net
                         (IWebSocketConnection Connection, CancellationTokenSource CancellationTokenSource, TStateType State)[] tuples;
 
                         var now = DateTime.UtcNow;
-                        await _lock.WaitAsync().ConfigureAwait(false);
-                        try
+                        lock (_activeConnectionsLock)
                         {
                             if (_activeConnections.Count == 0)
                             {
@@ -174,10 +168,6 @@ namespace MediaBrowser.Controller.Net
                                 })
                                 .ToArray();
                         }
-                        finally
-                        {
-                            _lock.Release();
-                        }
 
                         if (tuples.Length == 0)
                         {
@@ -240,8 +230,7 @@ namespace MediaBrowser.Controller.Net
         /// <param name="message">The message.</param>
         private void Stop(WebSocketMessageInfo message)
         {
-            _lock.Wait();
-            try
+            lock (_activeConnectionsLock)
             {
                 var connection = _activeConnections.FirstOrDefault(c => c.Connection == message.Connection);
 
@@ -250,10 +239,6 @@ namespace MediaBrowser.Controller.Net
                     DisposeConnection(connection);
                 }
             }
-            finally
-            {
-                _lock.Release();
-            }
         }
 
         /// <summary>
@@ -283,15 +268,10 @@ namespace MediaBrowser.Controller.Net
                 Logger.LogError(ex, "Error disposing websocket");
             }
 
-            _lock.Wait();
-            try
+            lock (_activeConnectionsLock)
             {
                 _activeConnections.Remove(connection);
             }
-            finally
-            {
-                _lock.Release();
-            }
         }
 
         protected virtual async ValueTask DisposeAsyncCore()
@@ -306,18 +286,13 @@ namespace MediaBrowser.Controller.Net
                 Logger.LogError(ex, "Disposing the message consumer failed");
             }
 
-            await _lock.WaitAsync().ConfigureAwait(false);
-            try
+            lock (_activeConnectionsLock)
             {
                 foreach (var connection in _activeConnections.ToArray())
                 {
                     DisposeConnection(connection);
                 }
             }
-            finally
-            {
-                _lock.Release();
-            }
         }
 
         /// <inheritdoc />