Synchronize access to data

Emby.Server.Implementations/Session/SessionWebSocketListener.cs

@@ -1,6 +1,5 @@
-using System.Collections.Generic;
 using System;
-using System.Collections.Concurrent;
+using System.Collections.Generic;
 using System.Linq;
 using System.Net.WebSockets;
 using System.Threading;
@@ -26,9 +25,9 @@ namespace Emby.Server.Implementations.Session
         public readonly int WebSocketLostTimeout = 60;
 
         /// <summary>
-        /// The keep-alive timer factor; controls how often the timer will check on the status of the WebSockets.
+        /// The keep-alive interval factor; controls how often the watcher will check on the status of the WebSockets.
         /// </summary>
-        public readonly double TimerFactor = 0.2;
+        public readonly double IntervalFactor = 0.2;
 
         /// <summary>
         /// The ForceKeepAlive factor; controls when a ForceKeepAlive is sent.
@@ -53,14 +52,24 @@ namespace Emby.Server.Implementations.Session
         private readonly IHttpServer _httpServer;
 
         /// <summary>
-        /// The KeepAlive timer.
+        /// The KeepAlive cancellation token.
         /// </summary>
-        private Timer _keepAliveTimer;
+        private CancellationTokenSource _keepAliveCancellationToken;
+
+        /// <summary>
+        /// Lock used for accesing the KeepAlive cancellation token.
+        /// </summary>
+        private readonly object _keepAliveLock = new object();
 
         /// <summary>
         /// The WebSocket watchlist.
         /// </summary>
-        private readonly ConcurrentDictionary<IWebSocketConnection, byte> _webSockets = new ConcurrentDictionary<IWebSocketConnection, byte>();
+        private readonly HashSet<IWebSocketConnection> _webSockets = new HashSet<IWebSocketConnection>();
+
+        /// <summary>
+        /// Lock used for accesing the WebSockets watchlist.
+        /// </summary>
+        private readonly object _webSocketsLock = new object();
 
         /// <summary>
         /// Initializes a new instance of the <see cref="SessionWebSocketListener" /> class.
@@ -113,7 +122,7 @@ namespace Emby.Server.Implementations.Session
         public void Dispose()
         {
             _httpServer.WebSocketConnected -= _serverManager_WebSocketConnected;
-            StopKeepAliveTimer();
+            StopKeepAlive();
         }
 
         /// <summary>
@@ -140,6 +149,7 @@ namespace Emby.Server.Implementations.Session
         private void OnWebSocketClosed(object sender, EventArgs e)
         {
             var webSocket = (IWebSocketConnection) sender;
+            _logger.LogDebug("WebSockets {0} closed.", webSocket);
             RemoveWebSocket(webSocket);
         }
 
@@ -147,9 +157,11 @@ namespace Emby.Server.Implementations.Session
         /// Adds a WebSocket to the KeepAlive watchlist.
         /// </summary>
         /// <param name="webSocket">The WebSocket to monitor.</param>
-        private async void KeepAliveWebSocket(IWebSocketConnection webSocket)
+        private async Task KeepAliveWebSocket(IWebSocketConnection webSocket)
         {
-            if (!_webSockets.TryAdd(webSocket, 0))
+            lock (_webSocketsLock)
+            {
+                if (!_webSockets.Add(webSocket))
                 {
                     _logger.LogWarning("Multiple attempts to keep alive single WebSocket {0}", webSocket);
                     return;
@@ -157,6 +169,9 @@ namespace Emby.Server.Implementations.Session
                 webSocket.Closed += OnWebSocketClosed;
                 webSocket.LastKeepAliveDate = DateTime.UtcNow;
 
+                StartKeepAlive();
+            }
+
             // Notify WebSocket about timeout
             try
             {
@@ -164,10 +179,8 @@ namespace Emby.Server.Implementations.Session
             }
             catch (WebSocketException exception)
             {
-                _logger.LogWarning(exception, "Error sending ForceKeepAlive message to WebSocket.");
+                _logger.LogWarning(exception, "Error sending ForceKeepAlive message to WebSocket {0}.", webSocket);
             }
-
-            StartKeepAliveTimer();
         }
 
         /// <summary>
@@ -175,60 +188,88 @@ namespace Emby.Server.Implementations.Session
         /// </summary>
         /// <param name="webSocket">The WebSocket to remove.</param>
         private void RemoveWebSocket(IWebSocketConnection webSocket)
+        {
+            lock (_webSocketsLock)
+            {
+                if (!_webSockets.Remove(webSocket))
+                {
+                    _logger.LogWarning("WebSocket {0} not on watchlist.", webSocket);
+                }
+                else
                 {
                     webSocket.Closed -= OnWebSocketClosed;
-            _webSockets.TryRemove(webSocket, out _);
                 }
-
-        /// <summary>
-        /// Starts the KeepAlive timer.
-        /// </summary>
-        private void StartKeepAliveTimer()
-        {
-            if (_keepAliveTimer == null)
-            {
-                _keepAliveTimer = new Timer(
-                    KeepAliveSockets,
-                    null,
-                    TimeSpan.FromSeconds(WebSocketLostTimeout * TimerFactor),
-                    TimeSpan.FromSeconds(WebSocketLostTimeout * TimerFactor)
-                );
             }
         }
 
         /// <summary>
-        /// Stops the KeepAlive timer.
+        /// Starts the KeepAlive watcher.
         /// </summary>
-        private void StopKeepAliveTimer()
+        private void StartKeepAlive()
         {
-            if (_keepAliveTimer != null)
+            lock (_keepAliveLock)
             {
-                _keepAliveTimer.Dispose();
+                if (_keepAliveCancellationToken == null)
-                _keepAliveTimer = null;
+                {
+                    _keepAliveCancellationToken = new CancellationTokenSource();
+                    // Start KeepAlive watcher
+                    KeepAliveSockets(
+                        TimeSpan.FromSeconds(WebSocketLostTimeout * IntervalFactor),
+                        _keepAliveCancellationToken.Token);
                 }
-
-            foreach (var pair in _webSockets)
-            {
-                pair.Key.Closed -= OnWebSocketClosed;
             }
         }
 
         /// <summary>
-        /// Checks status of KeepAlive of WebSockets.
+        /// Stops the KeepAlive watcher.
         /// </summary>
-        /// <param name="state">The state.</param>
+        private void StopKeepAlive()
-        private async void KeepAliveSockets(object state)
         {
-            var inactive = _webSockets.Keys.Where(i =>
+            lock (_keepAliveLock)
+            {
+                if (_keepAliveCancellationToken != null)
+                {
+                    _keepAliveCancellationToken.Cancel();
+                    _keepAliveCancellationToken = null;
+                }
+            }
+
+            lock (_webSocketsLock)
+            {
+                foreach (var webSocket in _webSockets)
+                {
+                    webSocket.Closed -= OnWebSocketClosed;
+                }
+                _webSockets.Clear();
+            }
+        }
+
+        /// <summary>
+        /// Checks status of KeepAlive of WebSockets once every the specified interval time.
+        /// </summary>
+        /// <param name="interval">The interval.</param>
+        /// <param name="cancellationToken">The cancellation token.</param>
+        private async Task KeepAliveSockets(TimeSpan interval, CancellationToken cancellationToken)
+        {
+            while (true)
+            {
+                _logger.LogDebug("Watching {0} WebSockets.", _webSockets.Count());
+
+                IEnumerable<IWebSocketConnection> inactive;
+                IEnumerable<IWebSocketConnection> lost;
+                lock (_webSocketsLock)
+                {
+                    inactive = _webSockets.Where(i =>
                     {
                         var elapsed = (DateTime.UtcNow - i.LastKeepAliveDate).TotalSeconds;
                         return (elapsed > WebSocketLostTimeout * ForceKeepAliveFactor) && (elapsed < WebSocketLostTimeout);
                     });
-            var lost = _webSockets.Keys.Where(i => (DateTime.UtcNow - i.LastKeepAliveDate).TotalSeconds >= WebSocketLostTimeout);
+                    lost = _webSockets.Where(i => (DateTime.UtcNow - i.LastKeepAliveDate).TotalSeconds >= WebSocketLostTimeout);
+                }
 
                 if (inactive.Any())
                 {
-                _logger.LogDebug("Sending ForceKeepAlive message to {0} inactive WebSockets.", inactive.Count());
+                    _logger.LogInformation("Sending ForceKeepAlive message to {0} inactive WebSockets.", inactive.Count());
                 }
 
                 foreach (var webSocket in inactive)
@@ -240,14 +281,16 @@ namespace Emby.Server.Implementations.Session
                     catch (WebSocketException exception)
                     {
                         _logger.LogInformation(exception, "Error sending ForceKeepAlive message to WebSocket.");
-                    lost.Append(webSocket);
+                        lost = lost.Append(webSocket);
                     }
                 }
 
+                lock (_webSocketsLock)
+                {
                     if (lost.Any())
                     {
                         _logger.LogInformation("Lost {0} WebSockets.", lost.Count());
-                foreach (var webSocket in lost)
+                        foreach (var webSocket in lost.ToList())
                         {
                             // TODO: handle session relative to the lost webSocket
                             RemoveWebSocket(webSocket);
@@ -256,7 +299,20 @@ namespace Emby.Server.Implementations.Session
 
                     if (!_webSockets.Any())
                     {
-                StopKeepAliveTimer();
+                        StopKeepAlive();
+                    }
+                }
+
+                // Wait for next interval
+                Task task = Task.Delay(interval, cancellationToken);
+                try
+                {
+                    await task;
+                }
+                catch (TaskCanceledException)
+                {
+                    return;
+                }
             }
         }
 

Emby.Server.Implementations/Syncplay/SyncplayManager.cs

@@ -1,5 +1,4 @@
 using System;
-using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Globalization;
 using System.Linq;
@@ -42,14 +41,19 @@ namespace Emby.Server.Implementations.Syncplay
         /// <summary>
         /// The map between sessions and groups.
         /// </summary>
-        private readonly ConcurrentDictionary<string, ISyncplayController> _sessionToGroupMap =
+        private readonly Dictionary<string, ISyncplayController> _sessionToGroupMap =
-            new ConcurrentDictionary<string, ISyncplayController>(StringComparer.OrdinalIgnoreCase);
+            new Dictionary<string, ISyncplayController>(StringComparer.OrdinalIgnoreCase);
 
         /// <summary>
         /// The groups.
         /// </summary>
-        private readonly ConcurrentDictionary<string, ISyncplayController> _groups =
+        private readonly Dictionary<string, ISyncplayController> _groups =
-            new ConcurrentDictionary<string, ISyncplayController>(StringComparer.OrdinalIgnoreCase);
+            new Dictionary<string, ISyncplayController>(StringComparer.OrdinalIgnoreCase);
+
+        /// <summary>
+        /// Lock used for accesing any group.
+        /// </summary>
+        private readonly object _groupsLock = new object();
 
         private bool _disposed = false;
 
@@ -175,6 +179,8 @@ namespace Emby.Server.Implementations.Syncplay
                 return;
             }
 
+            lock (_groupsLock)
+            {
                 if (IsSessionInGroup(session))
                 {
                     LeaveGroup(session);
@@ -185,6 +191,7 @@ namespace Emby.Server.Implementations.Syncplay
 
                 group.InitGroup(session);
             }
+        }
 
         /// <inheritdoc />
         public void JoinGroup(SessionInfo session, string groupId, JoinGroupRequest request)
@@ -203,6 +210,8 @@ namespace Emby.Server.Implementations.Syncplay
                 return;
             }
 
+            lock (_groupsLock)
+            {
                 ISyncplayController group;
                 _groups.TryGetValue(groupId, out group);
 
@@ -239,12 +248,14 @@ namespace Emby.Server.Implementations.Syncplay
 
                 group.SessionJoin(session, request);
             }
+        }
 
         /// <inheritdoc />
         public void LeaveGroup(SessionInfo session)
         {
             // TODO: determine what happens to users that are in a group and get their permissions revoked
-
+            lock (_groupsLock)
+            {
                 ISyncplayController group;
                 _sessionToGroupMap.TryGetValue(session.Id, out group);
 
@@ -259,6 +270,7 @@ namespace Emby.Server.Implementations.Syncplay
                     _sessionManager.SendSyncplayGroupUpdate(session.Id.ToString(), error, CancellationToken.None);
                     return;
                 }
+
                 group.SessionLeave(session);
 
                 if (group.IsGroupEmpty())
@@ -266,6 +278,7 @@ namespace Emby.Server.Implementations.Syncplay
                     _groups.Remove(group.GetGroupId().ToString(), out _);
                 }
             }
+        }
 
         /// <inheritdoc />
         public List<GroupInfoView> ListGroups(SessionInfo session)
@@ -314,6 +327,8 @@ namespace Emby.Server.Implementations.Syncplay
                 return;
             }
 
+            lock (_groupsLock)
+            {
                 ISyncplayController group;
                 _sessionToGroupMap.TryGetValue(session.Id, out group);
 
@@ -328,8 +343,10 @@ namespace Emby.Server.Implementations.Syncplay
                     _sessionManager.SendSyncplayGroupUpdate(session.Id.ToString(), error, CancellationToken.None);
                     return;
                 }
+
                 group.HandleRequest(session, request);
             }
+        }
 
         /// <inheritdoc />
         public void AddSessionToGroup(SessionInfo session, ISyncplayController group)

MediaBrowser.Api/Syncplay/TimeSyncService.cs

@@ -9,7 +9,6 @@ using Microsoft.Extensions.Logging;
 namespace MediaBrowser.Api.Syncplay
 {
     [Route("/GetUtcTime", "GET", Summary = "Get UtcTime")]
-    [Authenticated]
     public class GetUtcTime : IReturnVoid
     {
         // Nothing
@@ -33,13 +32,10 @@ namespace MediaBrowser.Api.Syncplay
         public TimeSyncService(
             ILogger<TimeSyncService> logger,
             IServerConfigurationManager serverConfigurationManager,
-            IHttpResultFactory httpResultFactory,
+            IHttpResultFactory httpResultFactory)
-            ISessionManager sessionManager,
-            ISessionContext sessionContext)
             : base(logger, serverConfigurationManager, httpResultFactory)
         {
-            _sessionManager = sessionManager;
+            // Do nothing
-            _sessionContext = sessionContext;
         }
 
         /// <summary>