From 976459d3e8a8b889cebc2cf281e38b0fbc19c9b9 Mon Sep 17 00:00:00 2001 From: Bond_009 Date: Tue, 17 Dec 2019 23:15:02 +0100 Subject: Rewrite WebSocket handling code --- .../Net/IWebSocketConnection.cs | 28 ++++------------------ 1 file changed, 4 insertions(+), 24 deletions(-) (limited to 'MediaBrowser.Controller/Net/IWebSocketConnection.cs') diff --git a/MediaBrowser.Controller/Net/IWebSocketConnection.cs b/MediaBrowser.Controller/Net/IWebSocketConnection.cs index 566897b31..e2a714d5b 100644 --- a/MediaBrowser.Controller/Net/IWebSocketConnection.cs +++ b/MediaBrowser.Controller/Net/IWebSocketConnection.cs @@ -1,9 +1,9 @@ using System; +using System.Net; using System.Net.WebSockets; using System.Threading; using System.Threading.Tasks; using MediaBrowser.Model.Net; -using MediaBrowser.Model.Services; using Microsoft.AspNetCore.Http; namespace MediaBrowser.Controller.Net @@ -15,12 +15,6 @@ namespace MediaBrowser.Controller.Net /// event EventHandler Closed; - /// - /// Gets the id. - /// - /// The id. - Guid Id { get; } - /// /// Gets the last activity date. /// @@ -32,6 +26,7 @@ namespace MediaBrowser.Controller.Net /// /// The URL. string Url { get; set; } + /// /// Gets or sets the query string. /// @@ -54,7 +49,7 @@ namespace MediaBrowser.Controller.Net /// Gets the remote end point. /// /// The remote end point. - string RemoteEndPoint { get; } + IPAddress RemoteEndPoint { get; } /// /// Sends a message asynchronously. @@ -66,21 +61,6 @@ namespace MediaBrowser.Controller.Net /// message Task SendAsync(WebSocketMessage message, CancellationToken cancellationToken); - /// - /// Sends a message asynchronously. - /// - /// The buffer. - /// The cancellation token. - /// Task. - Task SendAsync(byte[] buffer, CancellationToken cancellationToken); - - /// - /// Sends a message asynchronously. - /// - /// The text. - /// The cancellation token. - /// Task. - /// buffer - Task SendAsync(string text, CancellationToken cancellationToken); + Task ProcessAsync(CancellationToken cancellationToken = default); } } -- cgit v1.2.3 From 5ca68f9623e414b85ddbda1f97895f1b90bd05e0 Mon Sep 17 00:00:00 2001 From: Bond_009 Date: Thu, 26 Dec 2019 20:57:46 +0100 Subject: Fix nullref exception and added logging --- .../HttpServer/HttpListenerHost.cs | 17 +++--- .../HttpServer/WebSocketConnection.cs | 63 ++++++++-------------- .../Session/SessionManager.cs | 3 +- .../Session/SessionWebSocketListener.cs | 2 +- .../Session/WebSocketController.cs | 5 +- .../Net/IWebSocketConnection.cs | 16 +++--- 6 files changed, 41 insertions(+), 65 deletions(-) (limited to 'MediaBrowser.Controller/Net/IWebSocketConnection.cs') diff --git a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs index 4baf96ab5..ebae4d0b1 100644 --- a/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs +++ b/Emby.Server.Implementations/HttpServer/HttpListenerHost.cs @@ -518,30 +518,29 @@ namespace Emby.Server.Implementations.HttpServer return; } - var url = context.Request.GetDisplayUrl(); - _logger.LogInformation("WS {Url}. UserAgent: {UserAgent}", url, context.Request.Headers[HeaderNames.UserAgent].ToString()); - try { - var webSocket = await context.WebSockets.AcceptWebSocketAsync(null).ConfigureAwait(false); + _logger.LogInformation("WS Request from {IP}", context.Connection.RemoteIpAddress); + + WebSocket webSocket = await context.WebSockets.AcceptWebSocketAsync().ConfigureAwait(false); var connection = new WebSocketConnection( _loggerFactory.CreateLogger(), webSocket, - context.Connection.RemoteIpAddress) + context.Connection.RemoteIpAddress, + context.Request.Query) { - Url = url, - QueryString = context.Request.Query, OnReceive = ProcessWebSocketMessageReceived }; WebSocketConnected?.Invoke(this, new GenericEventArgs(connection)); await connection.ProcessAsync().ConfigureAwait(false); + _logger.LogInformation("WS closed from {IP}", context.Connection.RemoteIpAddress); } - catch (WebSocketException ex) + catch (Exception ex) // Otherwise ASP.Net will ignore the exception { - _logger.LogError(ex, "ProcessWebSocketRequest error"); + _logger.LogError(ex, "WebSocketRequestHandler error"); if (!context.Response.HasStarted) { context.Response.StatusCode = 500; diff --git a/Emby.Server.Implementations/HttpServer/WebSocketConnection.cs b/Emby.Server.Implementations/HttpServer/WebSocketConnection.cs index b4f420e5d..88974f9ab 100644 --- a/Emby.Server.Implementations/HttpServer/WebSocketConnection.cs +++ b/Emby.Server.Implementations/HttpServer/WebSocketConnection.cs @@ -1,4 +1,6 @@ -using System; +#nullable enable + +using System; using System.Buffers; using System.IO.Pipelines; using System.Net; @@ -39,47 +41,38 @@ namespace Emby.Server.Implementations.HttpServer /// /// Initializes a new instance of the class. /// + /// The logger. /// The socket. /// The remote end point. - /// The logger. - /// socket - public WebSocketConnection(ILogger logger, WebSocket socket, IPAddress remoteEndPoint) + /// The query. + public WebSocketConnection( + ILogger logger, + WebSocket socket, + IPAddress? remoteEndPoint, + IQueryCollection query) { - if (socket == null) - { - throw new ArgumentNullException(nameof(socket)); - } - - if (remoteEndPoint != null) - { - throw new ArgumentNullException(nameof(remoteEndPoint)); - } - - if (logger == null) - { - throw new ArgumentNullException(nameof(logger)); - } - + _logger = logger; _socket = socket; RemoteEndPoint = remoteEndPoint; - _logger = logger; + QueryString = query; _jsonOptions = JsonDefaults.GetOptions(); + LastActivityDate = DateTime.Now; } /// - public event EventHandler Closed; + public event EventHandler? Closed; /// /// Gets or sets the remote end point. /// - public IPAddress RemoteEndPoint { get; private set; } + public IPAddress? RemoteEndPoint { get; } /// /// Gets or sets the receive action. /// /// The receive action. - public Func OnReceive { get; set; } + public Func? OnReceive { get; set; } /// /// Gets the last activity date. @@ -87,17 +80,11 @@ namespace Emby.Server.Implementations.HttpServer /// The last activity date. public DateTime LastActivityDate { get; private set; } - /// - /// Gets or sets the URL. - /// - /// The URL. - public string Url { get; set; } - /// /// Gets or sets the query string. /// /// The query string. - public IQueryCollection QueryString { get; set; } + public IQueryCollection QueryString { get; } /// /// Gets the state. @@ -115,11 +102,6 @@ namespace Emby.Server.Implementations.HttpServer /// message public Task SendAsync(WebSocketMessage message, CancellationToken cancellationToken) { - if (message == null) - { - throw new ArgumentNullException(nameof(message)); - } - var json = JsonSerializer.SerializeToUtf8Bytes(message, _jsonOptions); return _socket.SendAsync(json, WebSocketMessageType.Text, true, cancellationToken); } @@ -140,7 +122,7 @@ namespace Emby.Server.Implementations.HttpServer int bytesRead = receiveresult.Count; if (bytesRead == 0) { - continue; + break; } // Tell the PipeWriter how much was read from the Socket @@ -154,6 +136,8 @@ namespace Emby.Server.Implementations.HttpServer break; } + LastActivityDate = DateTime.UtcNow; + if (receiveresult.EndOfMessage) { await ProcessInternal(pipe.Reader).ConfigureAwait(false); @@ -162,10 +146,7 @@ namespace Emby.Server.Implementations.HttpServer if (_socket.State == WebSocketState.Open) { - await _socket.CloseAsync( - WebSocketCloseStatus.NormalClosure, - string.Empty, // REVIEW: human readable explanation as to why the connection is closed. - cancellationToken).ConfigureAwait(false); + _logger.LogWarning("Stopped reading from websocket before it was closed"); } Closed?.Invoke(this, EventArgs.Empty); @@ -175,8 +156,6 @@ namespace Emby.Server.Implementations.HttpServer private async Task ProcessInternal(PipeReader reader) { - LastActivityDate = DateTime.UtcNow; - if (OnReceive == null) { return; diff --git a/Emby.Server.Implementations/Session/SessionManager.cs b/Emby.Server.Implementations/Session/SessionManager.cs index db00ceeb7..0d5df1dad 100644 --- a/Emby.Server.Implementations/Session/SessionManager.cs +++ b/Emby.Server.Implementations/Session/SessionManager.cs @@ -1726,6 +1726,7 @@ namespace Emby.Server.Implementations.Session string.Equals(i.Client, client)); } + /// public SessionInfo GetSessionByAuthenticationToken(AuthenticationInfo info, string deviceId, string remoteEndpoint, string appVersion) { if (info == null) @@ -1733,7 +1734,7 @@ namespace Emby.Server.Implementations.Session throw new ArgumentNullException(nameof(info)); } - var user = info.UserId.Equals(Guid.Empty) + var user = info.UserId == Guid.Empty ? null : _userManager.GetUserById(info.UserId); diff --git a/Emby.Server.Implementations/Session/SessionWebSocketListener.cs b/Emby.Server.Implementations/Session/SessionWebSocketListener.cs index 13b42698d..d4e4ba1f2 100644 --- a/Emby.Server.Implementations/Session/SessionWebSocketListener.cs +++ b/Emby.Server.Implementations/Session/SessionWebSocketListener.cs @@ -56,7 +56,7 @@ namespace Emby.Server.Implementations.Session } else { - _logger.LogWarning("Unable to determine session based on url: {0}", e.Argument.Url); + _logger.LogWarning("Unable to determine session based on query string: {0}", e.Argument.QueryString); } } diff --git a/Emby.Server.Implementations/Session/WebSocketController.cs b/Emby.Server.Implementations/Session/WebSocketController.cs index c17e67da9..536013c7a 100644 --- a/Emby.Server.Implementations/Session/WebSocketController.cs +++ b/Emby.Server.Implementations/Session/WebSocketController.cs @@ -53,11 +53,12 @@ namespace Emby.Server.Implementations.Session private void OnConnectionClosed(object sender, EventArgs e) { - _logger.LogDebug("Removing websocket from session {Session}", _session.Id); var connection = (IWebSocketConnection)sender; + _logger.LogDebug("Removing websocket from session {Session}", _session.Id); _sockets.Remove(connection); - _sessionManager.CloseIfNeeded(_session); + connection.Closed -= OnConnectionClosed; connection.Dispose(); + _sessionManager.CloseIfNeeded(_session); } /// diff --git a/MediaBrowser.Controller/Net/IWebSocketConnection.cs b/MediaBrowser.Controller/Net/IWebSocketConnection.cs index e2a714d5b..d5555884d 100644 --- a/MediaBrowser.Controller/Net/IWebSocketConnection.cs +++ b/MediaBrowser.Controller/Net/IWebSocketConnection.cs @@ -1,3 +1,5 @@ +#nullable enable + using System; using System.Net; using System.Net.WebSockets; @@ -13,7 +15,7 @@ namespace MediaBrowser.Controller.Net /// /// Occurs when [closed]. /// - event EventHandler Closed; + event EventHandler? Closed; /// /// Gets the last activity date. @@ -21,23 +23,17 @@ namespace MediaBrowser.Controller.Net /// The last activity date. DateTime LastActivityDate { get; } - /// - /// Gets or sets the URL. - /// - /// The URL. - string Url { get; set; } - /// /// Gets or sets the query string. /// /// The query string. - IQueryCollection QueryString { get; set; } + IQueryCollection QueryString { get; } /// /// Gets or sets the receive action. /// /// The receive action. - Func OnReceive { get; set; } + Func? OnReceive { get; set; } /// /// Gets the state. @@ -49,7 +45,7 @@ namespace MediaBrowser.Controller.Net /// Gets the remote end point. /// /// The remote end point. - IPAddress RemoteEndPoint { get; } + IPAddress? RemoteEndPoint { get; } /// /// Sends a message asynchronously. -- cgit v1.2.3 From 4d311870d2f40f67da6df5641b53df637fdee88d Mon Sep 17 00:00:00 2001 From: Bond-009 Date: Fri, 27 Dec 2019 14:42:53 +0100 Subject: Fix websocket handling --- .../HttpServer/WebSocketConnection.cs | 73 +++++++++------------- .../Session/WebSocketController.cs | 2 - .../Net/IWebSocketConnection.cs | 2 +- 3 files changed, 30 insertions(+), 47 deletions(-) (limited to 'MediaBrowser.Controller/Net/IWebSocketConnection.cs') diff --git a/Emby.Server.Implementations/HttpServer/WebSocketConnection.cs b/Emby.Server.Implementations/HttpServer/WebSocketConnection.cs index 88974f9ab..913a51217 100644 --- a/Emby.Server.Implementations/HttpServer/WebSocketConnection.cs +++ b/Emby.Server.Implementations/HttpServer/WebSocketConnection.cs @@ -99,7 +99,6 @@ namespace Emby.Server.Implementations.HttpServer /// The message. /// The cancellation token. /// Task. - /// message public Task SendAsync(WebSocketMessage message, CancellationToken cancellationToken) { var json = JsonSerializer.SerializeToUtf8Bytes(message, _jsonOptions); @@ -117,7 +116,6 @@ namespace Emby.Server.Implementations.HttpServer { // Allocate at least 512 bytes from the PipeWriter Memory memory = writer.GetMemory(512); - receiveresult = await _socket.ReceiveAsync(memory, cancellationToken); int bytesRead = receiveresult.Count; if (bytesRead == 0) @@ -144,33 +142,30 @@ namespace Emby.Server.Implementations.HttpServer } } while (_socket.State == WebSocketState.Open && receiveresult.MessageType != WebSocketMessageType.Close); - if (_socket.State == WebSocketState.Open) - { - _logger.LogWarning("Stopped reading from websocket before it was closed"); - } - Closed?.Invoke(this, EventArgs.Empty); - _socket.Dispose(); + await _socket.CloseAsync( + WebSocketCloseStatus.NormalClosure, + string.Empty, + cancellationToken).ConfigureAwait(false); } private async Task ProcessInternal(PipeReader reader) { + ReadResult result = await reader.ReadAsync().ConfigureAwait(false); + ReadOnlySequence buffer = result.Buffer; + if (OnReceive == null) { + // Tell the PipeReader how much of the buffer we have consumed + reader.AdvanceTo(buffer.End); return; } + WebSocketMessage stub; try { - var result = await reader.ReadAsync().ConfigureAwait(false); - if (!result.IsCompleted) - { - return; - } - WebSocketMessage stub; - var buffer = result.Buffer; if (buffer.IsSingleSegment) { stub = JsonSerializer.Deserialize>(buffer.FirstSpan, _jsonOptions); @@ -188,46 +183,36 @@ namespace Emby.Server.Implementations.HttpServer ArrayPool.Shared.Return(buf); } } - - var info = new WebSocketMessageInfo - { - MessageType = stub.MessageType, - Data = stub.Data.ToString(), - Connection = this - }; - - await OnReceive(info).ConfigureAwait(false); } catch (JsonException ex) { + // Tell the PipeReader how much of the buffer we have consumed + reader.AdvanceTo(buffer.End); _logger.LogError(ex, "Error processing web socket message"); + return; } - } - /// - public void Dispose() - { - Dispose(true); - GC.SuppressFinalize(this); - } + // Tell the PipeReader how much of the buffer we have consumed + reader.AdvanceTo(buffer.End); - /// - /// Releases unmanaged and - optionally - managed resources. - /// - /// true to release both managed and unmanaged resources; false to release only unmanaged resources. - protected virtual void Dispose(bool dispose) - { - if (_disposed) + _logger.LogDebug("WS received message: {@Message}", stub); + + var info = new WebSocketMessageInfo { - return; - } + MessageType = stub.MessageType, + Data = stub.Data?.ToString(), // Data can be null + Connection = this + }; + + _logger.LogDebug("WS message info: {@MessageInfo}", info); - if (dispose) + await OnReceive(info).ConfigureAwait(false); + + // Stop reading if there's no more data coming + if (result.IsCompleted) { - _socket.Dispose(); + return; } - - _disposed = true; } } } diff --git a/Emby.Server.Implementations/Session/WebSocketController.cs b/Emby.Server.Implementations/Session/WebSocketController.cs index 536013c7a..c3c4b716f 100644 --- a/Emby.Server.Implementations/Session/WebSocketController.cs +++ b/Emby.Server.Implementations/Session/WebSocketController.cs @@ -57,7 +57,6 @@ namespace Emby.Server.Implementations.Session _logger.LogDebug("Removing websocket from session {Session}", _session.Id); _sockets.Remove(connection); connection.Closed -= OnConnectionClosed; - connection.Dispose(); _sessionManager.CloseIfNeeded(_session); } @@ -96,7 +95,6 @@ namespace Emby.Server.Implementations.Session foreach (var socket in _sockets) { socket.Closed -= OnConnectionClosed; - socket.Dispose(); } _disposed = true; diff --git a/MediaBrowser.Controller/Net/IWebSocketConnection.cs b/MediaBrowser.Controller/Net/IWebSocketConnection.cs index d5555884d..09e43c683 100644 --- a/MediaBrowser.Controller/Net/IWebSocketConnection.cs +++ b/MediaBrowser.Controller/Net/IWebSocketConnection.cs @@ -10,7 +10,7 @@ using Microsoft.AspNetCore.Http; namespace MediaBrowser.Controller.Net { - public interface IWebSocketConnection : IDisposable + public interface IWebSocketConnection { /// /// Occurs when [closed]. -- cgit v1.2.3 From aad5058d25b3c295e9ea5b4330dde219034ba8c8 Mon Sep 17 00:00:00 2001 From: gion Date: Fri, 17 Apr 2020 13:47:00 +0200 Subject: Implement KeepAlive for WebSockets --- .../HttpServer/WebSocketConnection.cs | 27 +++- .../Session/SessionWebSocketListener.cs | 156 +++++++++++++++++++++ .../Net/IWebSocketConnection.cs | 6 + 3 files changed, 183 insertions(+), 6 deletions(-) (limited to 'MediaBrowser.Controller/Net/IWebSocketConnection.cs') diff --git a/Emby.Server.Implementations/HttpServer/WebSocketConnection.cs b/Emby.Server.Implementations/HttpServer/WebSocketConnection.cs index 2292d86a4..171047e65 100644 --- a/Emby.Server.Implementations/HttpServer/WebSocketConnection.cs +++ b/Emby.Server.Implementations/HttpServer/WebSocketConnection.cs @@ -94,6 +94,9 @@ namespace Emby.Server.Implementations.HttpServer /// The last activity date. public DateTime LastActivityDate { get; private set; } + /// + public DateTime LastKeepAliveDate { get; set; } + /// /// Gets the id. /// @@ -158,11 +161,6 @@ namespace Emby.Server.Implementations.HttpServer return; } - if (OnReceive == null) - { - return; - } - try { var stub = (WebSocketMessage)_jsonSerializer.DeserializeFromString(message, typeof(WebSocketMessage)); @@ -174,7 +172,15 @@ namespace Emby.Server.Implementations.HttpServer Connection = this }; - OnReceive(info); + if (info.MessageType.Equals("KeepAlive", StringComparison.Ordinal)) + { + SendKeepAliveResponse(); + } + + if (OnReceive != null) + { + OnReceive(info); + } } catch (Exception ex) { @@ -233,6 +239,15 @@ namespace Emby.Server.Implementations.HttpServer return _socket.SendAsync(text, true, cancellationToken); } + private Task SendKeepAliveResponse() + { + LastKeepAliveDate = DateTime.UtcNow; + return SendAsync(new WebSocketMessage + { + MessageType = "KeepAlive" + }, CancellationToken.None); + } + /// public void Dispose() { diff --git a/Emby.Server.Implementations/Session/SessionWebSocketListener.cs b/Emby.Server.Implementations/Session/SessionWebSocketListener.cs index 930f2d35d..d8e02ef39 100644 --- a/Emby.Server.Implementations/Session/SessionWebSocketListener.cs +++ b/Emby.Server.Implementations/Session/SessionWebSocketListener.cs @@ -1,8 +1,13 @@ using System; +using System.Collections.Concurrent; +using System.Linq; +using System.Net.WebSockets; +using System.Threading; using System.Threading.Tasks; using MediaBrowser.Controller.Net; using MediaBrowser.Controller.Session; using MediaBrowser.Model.Events; +using MediaBrowser.Model.Net; using MediaBrowser.Model.Serialization; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging; @@ -14,6 +19,21 @@ namespace Emby.Server.Implementations.Session /// public class SessionWebSocketListener : IWebSocketListener, IDisposable { + /// + /// The timeout in seconds after which a WebSocket is considered to be lost. + /// + public readonly int WebSocketLostTimeout = 60; + + /// + /// The timer factor; controls the frequency of the timer. + /// + public readonly double TimerFactor = 0.2; + + /// + /// The ForceKeepAlive factor; controls when a ForceKeepAlive is sent. + /// + public readonly double ForceKeepAliveFactor = 0.75; + /// /// The _session manager /// @@ -31,6 +51,15 @@ namespace Emby.Server.Implementations.Session private readonly IHttpServer _httpServer; + /// + /// The KeepAlive timer. + /// + private Timer _keepAliveTimer; + + /// + /// The WebSocket watchlist. + /// + private readonly ConcurrentDictionary _webSockets = new ConcurrentDictionary(); /// /// Initializes a new instance of the class. @@ -55,6 +84,7 @@ namespace Emby.Server.Implementations.Session if (session != null) { EnsureController(session, e.Argument); + KeepAliveWebSocket(e.Argument); } else { @@ -82,6 +112,7 @@ namespace Emby.Server.Implementations.Session public void Dispose() { _httpServer.WebSocketConnected -= _serverManager_WebSocketConnected; + StopKeepAliveTimer(); } /// @@ -99,5 +130,130 @@ namespace Emby.Server.Implementations.Session var controller = (WebSocketController)controllerInfo.Item1; controller.AddWebSocket(connection); } + + /// + /// Called when a WebSocket is closed. + /// + /// The WebSocket. + /// The event arguments. + private void _webSocket_Closed(object sender, EventArgs e) + { + var webSocket = (IWebSocketConnection) sender; + webSocket.Closed -= _webSocket_Closed; + _webSockets.TryRemove(webSocket, out _); + } + + /// + /// Adds a WebSocket to the KeepAlive watchlist. + /// + /// The WebSocket to monitor. + private async void KeepAliveWebSocket(IWebSocketConnection webSocket) + { + _webSockets.TryAdd(webSocket, 0); + webSocket.Closed += _webSocket_Closed; + webSocket.LastKeepAliveDate = DateTime.UtcNow; + + // Notify WebSocket about timeout + try + { + await SendForceKeepAlive(webSocket); + } + catch (WebSocketException exception) + { + _logger.LogDebug(exception, "Error sending ForceKeepAlive message to WebSocket."); + } + + StartKeepAliveTimer(); + } + + /// + /// Starts the KeepAlive timer. + /// + private void StartKeepAliveTimer() + { + if (_keepAliveTimer == null) + { + _keepAliveTimer = new Timer( + KeepAliveSockets, + null, + TimeSpan.FromSeconds(WebSocketLostTimeout * TimerFactor), + TimeSpan.FromSeconds(WebSocketLostTimeout * TimerFactor) + ); + } + } + + /// + /// Stops the KeepAlive timer. + /// + private void StopKeepAliveTimer() + { + if (_keepAliveTimer != null) + { + _keepAliveTimer.Dispose(); + _keepAliveTimer = null; + } + + foreach (var pair in _webSockets) + { + pair.Key.Closed -= _webSocket_Closed; + } + } + + /// + /// Checks status of KeepAlive of WebSockets. + /// + /// The state. + private async void KeepAliveSockets(object state) + { + var inactive = _webSockets.Keys.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); + + if (inactive.Any()) + { + _logger.LogDebug("Sending ForceKeepAlive message to {0} WebSockets.", inactive.Count()); + } + + foreach (var webSocket in inactive) + { + try + { + await SendForceKeepAlive(webSocket); + } + catch (WebSocketException exception) + { + _logger.LogDebug(exception, "Error sending ForceKeepAlive message to WebSocket."); + lost.Append(webSocket); + } + } + + if (lost.Any()) + { + // TODO: handle lost webSockets + _logger.LogDebug("Lost {0} WebSockets.", lost.Count()); + } + + if (!_webSockets.Any()) + { + StopKeepAliveTimer(); + } + } + + /// + /// Sends a ForceKeepAlive message to a WebSocket. + /// + /// The WebSocket. + /// Task. + private Task SendForceKeepAlive(IWebSocketConnection webSocket) + { + return webSocket.SendAsync(new WebSocketMessage + { + MessageType = "ForceKeepAlive", + Data = WebSocketLostTimeout + }, CancellationToken.None); + } } } diff --git a/MediaBrowser.Controller/Net/IWebSocketConnection.cs b/MediaBrowser.Controller/Net/IWebSocketConnection.cs index 31eb7ccb7..fb766ab57 100644 --- a/MediaBrowser.Controller/Net/IWebSocketConnection.cs +++ b/MediaBrowser.Controller/Net/IWebSocketConnection.cs @@ -26,6 +26,12 @@ namespace MediaBrowser.Controller.Net /// The last activity date. DateTime LastActivityDate { get; } + /// + /// Gets or sets the date of last Keeplive received. + /// + /// The date of last Keeplive received. + public DateTime LastKeepAliveDate { get; set; } + /// /// Gets or sets the URL. /// -- cgit v1.2.3 From 5c8cbd4087261f13d003d7d4eab082cbf335b4d4 Mon Sep 17 00:00:00 2001 From: gion Date: Sat, 9 May 2020 14:34:07 +0200 Subject: Fix code issues --- .../Session/SessionWebSocketListener.cs | 21 +++++----- .../SyncPlay/SyncPlayController.cs | 4 +- .../SyncPlay/SyncPlayManager.cs | 28 +++++++++---- MediaBrowser.Api/SyncPlay/SyncPlayService.cs | 47 +++++++++++----------- .../Net/IWebSocketConnection.cs | 2 +- MediaBrowser.Controller/SyncPlay/GroupInfo.cs | 32 +++++++++++---- .../SyncPlay/ISyncPlayManager.cs | 2 +- MediaBrowser.Model/SyncPlay/GroupInfoView.cs | 4 +- 8 files changed, 86 insertions(+), 54 deletions(-) (limited to 'MediaBrowser.Controller/Net/IWebSocketConnection.cs') diff --git a/Emby.Server.Implementations/Session/SessionWebSocketListener.cs b/Emby.Server.Implementations/Session/SessionWebSocketListener.cs index d1ee22ea8..3704445ab 100644 --- a/Emby.Server.Implementations/Session/SessionWebSocketListener.cs +++ b/Emby.Server.Implementations/Session/SessionWebSocketListener.cs @@ -22,17 +22,17 @@ namespace Emby.Server.Implementations.Session /// /// The timeout in seconds after which a WebSocket is considered to be lost. /// - public readonly int WebSocketLostTimeout = 60; + public const int WebSocketLostTimeout = 60; /// /// The keep-alive interval factor; controls how often the watcher will check on the status of the WebSockets. /// - public readonly double IntervalFactor = 0.2; + public const float IntervalFactor = 0.2f; /// /// The ForceKeepAlive factor; controls when a ForceKeepAlive is sent. /// - public readonly double ForceKeepAliveFactor = 0.75; + public const float ForceKeepAliveFactor = 0.75f; /// /// The _session manager @@ -213,7 +213,7 @@ namespace Emby.Server.Implementations.Session { _keepAliveCancellationToken = new CancellationTokenSource(); // Start KeepAlive watcher - var task = RepeatAsyncCallbackEvery( + _ = RepeatAsyncCallbackEvery( KeepAliveSockets, TimeSpan.FromSeconds(WebSocketLostTimeout * IntervalFactor), _keepAliveCancellationToken.Token); @@ -241,6 +241,7 @@ namespace Emby.Server.Implementations.Session { webSocket.Closed -= OnWebSocketClosed; } + _webSockets.Clear(); } } @@ -250,8 +251,8 @@ namespace Emby.Server.Implementations.Session /// private async Task KeepAliveSockets() { - IEnumerable inactive; - IEnumerable lost; + List inactive; + List lost; lock (_webSocketsLock) { @@ -261,8 +262,8 @@ namespace Emby.Server.Implementations.Session { var elapsed = (DateTime.UtcNow - i.LastKeepAliveDate).TotalSeconds; return (elapsed > WebSocketLostTimeout * ForceKeepAliveFactor) && (elapsed < WebSocketLostTimeout); - }); - lost = _webSockets.Where(i => (DateTime.UtcNow - i.LastKeepAliveDate).TotalSeconds >= WebSocketLostTimeout); + }).ToList(); + lost = _webSockets.Where(i => (DateTime.UtcNow - i.LastKeepAliveDate).TotalSeconds >= WebSocketLostTimeout).ToList(); } if (inactive.Any()) @@ -279,7 +280,7 @@ namespace Emby.Server.Implementations.Session catch (WebSocketException exception) { _logger.LogInformation(exception, "Error sending ForceKeepAlive message to WebSocket."); - lost = lost.Append(webSocket); + lost.Add(webSocket); } } @@ -288,7 +289,7 @@ namespace Emby.Server.Implementations.Session if (lost.Any()) { _logger.LogInformation("Lost {0} WebSockets.", lost.Count()); - foreach (var webSocket in lost.ToList()) + foreach (var webSocket in lost) { // TODO: handle session relative to the lost webSocket RemoveWebSocket(webSocket); diff --git a/Emby.Server.Implementations/SyncPlay/SyncPlayController.cs b/Emby.Server.Implementations/SyncPlay/SyncPlayController.cs index 9c9758de1..c7bd242a7 100644 --- a/Emby.Server.Implementations/SyncPlay/SyncPlayController.cs +++ b/Emby.Server.Implementations/SyncPlay/SyncPlayController.cs @@ -144,7 +144,7 @@ namespace Emby.Server.Implementations.SyncPlay session => session.Session ).ToArray(); default: - return new SessionInfo[] { }; + return Array.Empty(); } } @@ -541,7 +541,7 @@ namespace Emby.Server.Implementations.SyncPlay PlayingItemName = _group.PlayingItem.Name, PlayingItemId = _group.PlayingItem.Id.ToString(), PositionTicks = _group.PositionTicks, - Participants = _group.Participants.Values.Select(session => session.Session.UserName).Distinct().ToArray() + Participants = _group.Participants.Values.Select(session => session.Session.UserName).Distinct().ToList().AsReadOnly() }; } } diff --git a/Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs b/Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs index d3197d97b..93cec1304 100644 --- a/Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs +++ b/Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs @@ -47,8 +47,8 @@ namespace Emby.Server.Implementations.SyncPlay /// /// The groups. /// - private readonly Dictionary _groups = - new Dictionary(StringComparer.OrdinalIgnoreCase); + private readonly Dictionary _groups = + new Dictionary(); /// /// Lock used for accesing any group. @@ -113,14 +113,22 @@ namespace Emby.Server.Implementations.SyncPlay private void OnSessionManagerSessionEnded(object sender, SessionEventArgs e) { var session = e.SessionInfo; - if (!IsSessionInGroup(session)) return; + if (!IsSessionInGroup(session)) + { + return; + } + LeaveGroup(session, CancellationToken.None); } private void OnSessionManagerPlaybackStopped(object sender, PlaybackStopEventArgs e) { var session = e.Session; - if (!IsSessionInGroup(session)) return; + if (!IsSessionInGroup(session)) + { + return; + } + LeaveGroup(session, CancellationToken.None); } @@ -193,14 +201,14 @@ namespace Emby.Server.Implementations.SyncPlay } var group = new SyncPlayController(_sessionManager, this); - _groups[group.GetGroupId().ToString()] = group; + _groups[group.GetGroupId()] = group; group.InitGroup(session, cancellationToken); } } /// - public void JoinGroup(SessionInfo session, string groupId, JoinGroupRequest request, CancellationToken cancellationToken) + public void JoinGroup(SessionInfo session, Guid groupId, JoinGroupRequest request, CancellationToken cancellationToken) { var user = _userManager.GetUserById(session.UserId); @@ -248,7 +256,11 @@ namespace Emby.Server.Implementations.SyncPlay if (IsSessionInGroup(session)) { - if (GetSessionGroup(session).Equals(groupId)) return; + if (GetSessionGroup(session).Equals(groupId)) + { + return; + } + LeaveGroup(session, cancellationToken); } @@ -282,7 +294,7 @@ namespace Emby.Server.Implementations.SyncPlay if (group.IsGroupEmpty()) { _logger.LogInformation("LeaveGroup: removing empty group {0}.", group.GetGroupId()); - _groups.Remove(group.GetGroupId().ToString(), out _); + _groups.Remove(group.GetGroupId(), out _); } } } diff --git a/MediaBrowser.Api/SyncPlay/SyncPlayService.cs b/MediaBrowser.Api/SyncPlay/SyncPlayService.cs index bcdc833e4..9137faf9f 100644 --- a/MediaBrowser.Api/SyncPlay/SyncPlayService.cs +++ b/MediaBrowser.Api/SyncPlay/SyncPlayService.cs @@ -171,31 +171,35 @@ namespace MediaBrowser.Api.SyncPlay public void Post(SyncPlayJoinGroup request) { var currentSession = GetSession(_sessionContext); - var joinRequest = new JoinGroupRequest() + + Guid groupId; + Guid playingItemId = Guid.Empty; + + var valid = Guid.TryParse(request.GroupId, out groupId); + if (!valid) { - GroupId = Guid.Parse(request.GroupId) - }; + Logger.LogError("JoinGroup: {0} is not a valid format for GroupId. Ignoring request.", request.GroupId); + return; + } // Both null and empty strings mean that client isn't playing anything if (!String.IsNullOrEmpty(request.PlayingItemId)) { - try - { - joinRequest.PlayingItemId = Guid.Parse(request.PlayingItemId); - } - catch (ArgumentNullException) - { - // Should never happen, but just in case - Logger.LogError("JoinGroup: null value for PlayingItemId. Ignoring request."); - return; - } - catch (FormatException) + valid = Guid.TryParse(request.PlayingItemId, out playingItemId); + if (!valid) { Logger.LogError("JoinGroup: {0} is not a valid format for PlayingItemId. Ignoring request.", request.PlayingItemId); return; } } - _syncPlayManager.JoinGroup(currentSession, request.GroupId, joinRequest, CancellationToken.None); + + var joinRequest = new JoinGroupRequest() + { + GroupId = groupId, + PlayingItemId = playingItemId + }; + + _syncPlayManager.JoinGroup(currentSession, groupId, joinRequest, CancellationToken.None); } /// @@ -217,21 +221,16 @@ namespace MediaBrowser.Api.SyncPlay { var currentSession = GetSession(_sessionContext); var filterItemId = Guid.Empty; + if (!String.IsNullOrEmpty(request.FilterItemId)) { - try - { - filterItemId = Guid.Parse(request.FilterItemId); - } - catch (ArgumentNullException) - { - Logger.LogWarning("ListGroups: null value for FilterItemId. Ignoring filter."); - } - catch (FormatException) + var valid = Guid.TryParse(request.FilterItemId, out filterItemId); + if (!valid) { Logger.LogWarning("ListGroups: {0} is not a valid format for FilterItemId. Ignoring filter.", request.FilterItemId); } } + return _syncPlayManager.ListGroups(currentSession, filterItemId); } diff --git a/MediaBrowser.Controller/Net/IWebSocketConnection.cs b/MediaBrowser.Controller/Net/IWebSocketConnection.cs index fb766ab57..b371a59e9 100644 --- a/MediaBrowser.Controller/Net/IWebSocketConnection.cs +++ b/MediaBrowser.Controller/Net/IWebSocketConnection.cs @@ -30,7 +30,7 @@ namespace MediaBrowser.Controller.Net /// Gets or sets the date of last Keeplive received. /// /// The date of last Keeplive received. - public DateTime LastKeepAliveDate { get; set; } + DateTime LastKeepAliveDate { get; set; } /// /// Gets or sets the URL. diff --git a/MediaBrowser.Controller/SyncPlay/GroupInfo.cs b/MediaBrowser.Controller/SyncPlay/GroupInfo.cs index 087748de0..bda49bd1b 100644 --- a/MediaBrowser.Controller/SyncPlay/GroupInfo.cs +++ b/MediaBrowser.Controller/SyncPlay/GroupInfo.cs @@ -69,7 +69,11 @@ namespace MediaBrowser.Controller.SyncPlay /// The session. public void AddSession(SessionInfo session) { - if (ContainsSession(session.Id.ToString())) return; + if (ContainsSession(session.Id.ToString())) + { + return; + } + var member = new GroupMember(); member.Session = session; member.Ping = DefaulPing; @@ -84,9 +88,12 @@ namespace MediaBrowser.Controller.SyncPlay public void RemoveSession(SessionInfo session) { - if (!ContainsSession(session.Id.ToString())) return; - GroupMember member; - Participants.Remove(session.Id.ToString(), out member); + if (!ContainsSession(session.Id.ToString())) + { + return; + } + + Participants.Remove(session.Id.ToString(), out _); } /// @@ -96,7 +103,11 @@ namespace MediaBrowser.Controller.SyncPlay /// The ping. public void UpdatePing(SessionInfo session, long ping) { - if (!ContainsSession(session.Id.ToString())) return; + if (!ContainsSession(session.Id.ToString())) + { + return; + } + Participants[session.Id.ToString()].Ping = ping; } @@ -121,7 +132,11 @@ namespace MediaBrowser.Controller.SyncPlay /// The state. public void SetBuffering(SessionInfo session, bool isBuffering) { - if (!ContainsSession(session.Id.ToString())) return; + if (!ContainsSession(session.Id.ToString())) + { + return; + } + Participants[session.Id.ToString()].IsBuffering = isBuffering; } @@ -133,7 +148,10 @@ namespace MediaBrowser.Controller.SyncPlay { foreach (var session in Participants.Values) { - if (session.IsBuffering) return true; + if (session.IsBuffering) + { + return true; + } } return false; } diff --git a/MediaBrowser.Controller/SyncPlay/ISyncPlayManager.cs b/MediaBrowser.Controller/SyncPlay/ISyncPlayManager.cs index 6c962ec85..006fb687b 100644 --- a/MediaBrowser.Controller/SyncPlay/ISyncPlayManager.cs +++ b/MediaBrowser.Controller/SyncPlay/ISyncPlayManager.cs @@ -25,7 +25,7 @@ namespace MediaBrowser.Controller.SyncPlay /// The group id. /// The request. /// The cancellation token. - void JoinGroup(SessionInfo session, string groupId, JoinGroupRequest request, CancellationToken cancellationToken); + void JoinGroup(SessionInfo session, Guid groupId, JoinGroupRequest request, CancellationToken cancellationToken); /// /// Removes the session from a group. diff --git a/MediaBrowser.Model/SyncPlay/GroupInfoView.cs b/MediaBrowser.Model/SyncPlay/GroupInfoView.cs index 7b833506b..f28ecf16d 100644 --- a/MediaBrowser.Model/SyncPlay/GroupInfoView.cs +++ b/MediaBrowser.Model/SyncPlay/GroupInfoView.cs @@ -1,3 +1,5 @@ +using System.Collections.Generic; + namespace MediaBrowser.Model.SyncPlay { /// @@ -33,6 +35,6 @@ namespace MediaBrowser.Model.SyncPlay /// Gets or sets the participants. /// /// The participants. - public string[] Participants { get; set; } + public IReadOnlyList Participants { get; set; } } } -- cgit v1.2.3