From 49bb5a6442ac8b0ddaff7958acedd43e1a72137c Mon Sep 17 00:00:00 2001
From: JPVenson <github@jpb.email>
Date: Tue, 4 Feb 2025 14:59:06 +0100
Subject: [PATCH] Merge pull request #13459 from
 JPVenson/bugfix/13457_FixWebSocketControllerConcurrency

Fixed Websocket not locking state correctly
---
 .../Session/WebSocketController.cs            | 98 +++++++++++++++----
 1 file changed, 81 insertions(+), 17 deletions(-)

diff --git a/Emby.Server.Implementations/Session/WebSocketController.cs b/Emby.Server.Implementations/Session/WebSocketController.cs
index cf8e0fb006..c45a4a60f5 100644
--- a/Emby.Server.Implementations/Session/WebSocketController.cs
+++ b/Emby.Server.Implementations/Session/WebSocketController.cs
@@ -21,6 +21,7 @@ namespace Emby.Server.Implementations.Session
         private readonly SessionInfo _session;
 
         private readonly List<IWebSocketConnection> _sockets;
+        private readonly ReaderWriterLockSlim _socketsLock;
         private bool _disposed = false;
 
         public WebSocketController(
@@ -31,10 +32,26 @@ namespace Emby.Server.Implementations.Session
             _logger = logger;
             _session = session;
             _sessionManager = sessionManager;
-            _sockets = new List<IWebSocketConnection>();
+            _sockets = new();
+            _socketsLock = new();
         }
 
-        private bool HasOpenSockets => GetActiveSockets().Any();
+        private bool HasOpenSockets
+        {
+            get
+            {
+                ObjectDisposedException.ThrowIf(_disposed, this);
+                try
+                {
+                    _socketsLock.EnterReadLock();
+                    return _sockets.Any(i => i.State == WebSocketState.Open);
+                }
+                finally
+                {
+                    _socketsLock.ExitReadLock();
+                }
+            }
+        }
 
         /// <inheritdoc />
         public bool SupportsMediaControl => HasOpenSockets;
@@ -42,23 +59,38 @@ namespace Emby.Server.Implementations.Session
         /// <inheritdoc />
         public bool IsSessionActive => HasOpenSockets;
 
-        private IEnumerable<IWebSocketConnection> GetActiveSockets()
-            => _sockets.Where(i => i.State == WebSocketState.Open);
-
         public void AddWebSocket(IWebSocketConnection connection)
         {
             _logger.LogDebug("Adding websocket to session {Session}", _session.Id);
-            _sockets.Add(connection);
-
-            connection.Closed += OnConnectionClosed;
+            ObjectDisposedException.ThrowIf(_disposed, this);
+            try
+            {
+                _socketsLock.EnterWriteLock();
+                _sockets.Add(connection);
+                connection.Closed += OnConnectionClosed;
+            }
+            finally
+            {
+                _socketsLock.ExitWriteLock();
+            }
         }
 
         private async void OnConnectionClosed(object? sender, EventArgs e)
         {
             var connection = sender as IWebSocketConnection ?? throw new ArgumentException($"{nameof(sender)} is not of type {nameof(IWebSocketConnection)}", nameof(sender));
             _logger.LogDebug("Removing websocket from session {Session}", _session.Id);
-            _sockets.Remove(connection);
-            connection.Closed -= OnConnectionClosed;
+            ObjectDisposedException.ThrowIf(_disposed, this);
+            try
+            {
+                _socketsLock.EnterWriteLock();
+                _sockets.Remove(connection);
+                connection.Closed -= OnConnectionClosed;
+            }
+            finally
+            {
+                _socketsLock.ExitWriteLock();
+            }
+
             await _sessionManager.CloseIfNeededAsync(_session).ConfigureAwait(false);
         }
 
@@ -69,7 +101,17 @@ namespace Emby.Server.Implementations.Session
             T data,
             CancellationToken cancellationToken)
         {
-            var socket = GetActiveSockets().MaxBy(i => i.LastActivityDate);
+            ObjectDisposedException.ThrowIf(_disposed, this);
+            IWebSocketConnection? socket;
+            try
+            {
+                _socketsLock.EnterReadLock();
+                socket = _sockets.Where(i => i.State == WebSocketState.Open).MaxBy(i => i.LastActivityDate);
+            }
+            finally
+            {
+                _socketsLock.ExitReadLock();
+            }
 
             if (socket is null)
             {
@@ -94,12 +136,23 @@ namespace Emby.Server.Implementations.Session
                 return;
             }
 
-            foreach (var socket in _sockets)
+            try
+            {
+                _socketsLock.EnterWriteLock();
+                foreach (var socket in _sockets)
+                {
+                    socket.Closed -= OnConnectionClosed;
+                    socket.Dispose();
+                }
+
+                _sockets.Clear();
+            }
+            finally
             {
-                socket.Closed -= OnConnectionClosed;
-                socket.Dispose();
+                _socketsLock.ExitWriteLock();
             }
 
+            _socketsLock.Dispose();
             _disposed = true;
         }
 
@@ -110,12 +163,23 @@ namespace Emby.Server.Implementations.Session
                 return;
             }
 
-            foreach (var socket in _sockets)
+            try
+            {
+                _socketsLock.EnterWriteLock();
+                foreach (var socket in _sockets)
+                {
+                    socket.Closed -= OnConnectionClosed;
+                    await socket.DisposeAsync().ConfigureAwait(false);
+                }
+
+                _sockets.Clear();
+            }
+            finally
             {
-                socket.Closed -= OnConnectionClosed;
-                await socket.DisposeAsync().ConfigureAwait(false);
+                _socketsLock.ExitWriteLock();
             }
 
+            _socketsLock.Dispose();
             _disposed = true;
         }
     }