From 2b6f3e940d54f369c432456402dc0763388deae6 Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Tue, 15 Aug 2017 09:27:39 -0700 Subject: [PATCH 1/6] Option 1 --- .../HttpConnection.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.AspNetCore.Sockets.Client.Http/HttpConnection.cs b/src/Microsoft.AspNetCore.Sockets.Client.Http/HttpConnection.cs index 30f9041a83..82f4ff3f89 100644 --- a/src/Microsoft.AspNetCore.Sockets.Client.Http/HttpConnection.cs +++ b/src/Microsoft.AspNetCore.Sockets.Client.Http/HttpConnection.cs @@ -409,14 +409,14 @@ public async Task DisposeAsync() // _startTask is returned to the user and they should handle exceptions. } - if (_transportChannel != null) + if (_transport != null) { - Output.TryComplete(); + await _transport.StopAsync(); } - if (_transport != null) + if (_transportChannel != null) { - await _transport.StopAsync(); + Output.TryComplete(); } if (_receiveLoopTask != null) From 740c58bbdbaeac7cc99f06440abff44c60e86e90 Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Tue, 15 Aug 2017 09:43:34 -0700 Subject: [PATCH 2/6] Option 2 --- .../HttpConnection.cs | 8 ++++---- .../WebSocketsTransport.cs | 6 ++++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.AspNetCore.Sockets.Client.Http/HttpConnection.cs b/src/Microsoft.AspNetCore.Sockets.Client.Http/HttpConnection.cs index 82f4ff3f89..30f9041a83 100644 --- a/src/Microsoft.AspNetCore.Sockets.Client.Http/HttpConnection.cs +++ b/src/Microsoft.AspNetCore.Sockets.Client.Http/HttpConnection.cs @@ -409,14 +409,14 @@ public async Task DisposeAsync() // _startTask is returned to the user and they should handle exceptions. } - if (_transport != null) + if (_transportChannel != null) { - await _transport.StopAsync(); + Output.TryComplete(); } - if (_transportChannel != null) + if (_transport != null) { - Output.TryComplete(); + await _transport.StopAsync(); } if (_receiveLoopTask != null) diff --git a/src/Microsoft.AspNetCore.Sockets.Client.Http/WebSocketsTransport.cs b/src/Microsoft.AspNetCore.Sockets.Client.Http/WebSocketsTransport.cs index 7177993346..fc36cdd2da 100644 --- a/src/Microsoft.AspNetCore.Sockets.Client.Http/WebSocketsTransport.cs +++ b/src/Microsoft.AspNetCore.Sockets.Client.Http/WebSocketsTransport.cs @@ -19,6 +19,7 @@ public class WebSocketsTransport : ITransport private readonly ClientWebSocket _webSocket = new ClientWebSocket(); private Channel _application; private readonly CancellationTokenSource _transportCts = new CancellationTokenSource(); + private readonly CancellationTokenSource _receiveCts = new CancellationTokenSource(); private readonly ILogger _logger; private string _connectionId; @@ -91,7 +92,7 @@ private async Task ReceiveMessages(Uri pollUrl) var buffer = new ArraySegment(new byte[bufferSize]); //Exceptions are handled above where the send and receive tasks are being run. - receiveResult = await _webSocket.ReceiveAsync(buffer, _transportCts.Token); + receiveResult = await _webSocket.ReceiveAsync(buffer, _receiveCts.Token); if (receiveResult.MessageType == WebSocketMessageType.Close) { _logger.WebSocketClosed(_connectionId, receiveResult.CloseStatus); @@ -248,7 +249,8 @@ private async Task CloseWebSocket() await _webSocket.CloseOutputAsync(WebSocketCloseStatus.NormalClosure, null, CancellationToken.None); // shutdown the transport after a timeout in case the server does not send close frame - _transportCts.CancelAfter(TimeSpan.FromSeconds(5)); + _receiveCts.CancelAfter(TimeSpan.FromSeconds(5)); + _transportCts.Cancel(); } } catch (Exception ex) From a5218bf772ef6282383962dd03cfef154688d5fa Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Wed, 16 Aug 2017 09:58:24 -0700 Subject: [PATCH 3/6] :( --- .../WebSocketsTransportTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Microsoft.AspNetCore.SignalR.Tests/WebSocketsTransportTests.cs b/test/Microsoft.AspNetCore.SignalR.Tests/WebSocketsTransportTests.cs index 627b03242b..62ef0b3e91 100644 --- a/test/Microsoft.AspNetCore.SignalR.Tests/WebSocketsTransportTests.cs +++ b/test/Microsoft.AspNetCore.SignalR.Tests/WebSocketsTransportTests.cs @@ -62,7 +62,7 @@ public async Task WebSocketsTransportStopsWhenConnectionChannelClosed() await webSocketsTransport.StartAsync(new Uri(_serverFixture.WebSocketsUrl + "/echo"), channelConnection, TransferMode.Binary, connectionId: string.Empty); connectionToTransport.Out.TryComplete(); - await webSocketsTransport.Running.OrTimeout(); + await webSocketsTransport.Running.OrTimeout(TimeSpan.FromSeconds(10)); } } From 73b5893e8d84b1e0f87f5d72de955f33881b8d8c Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Wed, 16 Aug 2017 10:38:29 -0700 Subject: [PATCH 4/6] fix --- .../WebSocketsTransport.cs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.AspNetCore.Sockets.Client.Http/WebSocketsTransport.cs b/src/Microsoft.AspNetCore.Sockets.Client.Http/WebSocketsTransport.cs index fc36cdd2da..327f1c4810 100644 --- a/src/Microsoft.AspNetCore.Sockets.Client.Http/WebSocketsTransport.cs +++ b/src/Microsoft.AspNetCore.Sockets.Client.Http/WebSocketsTransport.cs @@ -199,7 +199,7 @@ private async Task SendMessages(Uri sendUrl) finally { _logger.SendStopped(_connectionId); - _transportCts.Cancel(); + TriggerCancel(); } } @@ -249,8 +249,7 @@ private async Task CloseWebSocket() await _webSocket.CloseOutputAsync(WebSocketCloseStatus.NormalClosure, null, CancellationToken.None); // shutdown the transport after a timeout in case the server does not send close frame - _receiveCts.CancelAfter(TimeSpan.FromSeconds(5)); - _transportCts.Cancel(); + TriggerCancel(); } } catch (Exception ex) @@ -260,5 +259,12 @@ private async Task CloseWebSocket() _logger.ClosingWebSocketFailed(_connectionId, ex); } } + + private void TriggerCancel() + { + // Give server 5 seconds to respond with a close frame for graceful close. + _receiveCts.CancelAfter(TimeSpan.FromSeconds(5)); + _transportCts.Cancel(); + } } } From 475697fed643fbfed8df7aea31640abd30e03b36 Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Thu, 17 Aug 2017 16:30:02 -0700 Subject: [PATCH 5/6] fb --- .../WebSocketsTransport.cs | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.AspNetCore.Sockets.Client.Http/WebSocketsTransport.cs b/src/Microsoft.AspNetCore.Sockets.Client.Http/WebSocketsTransport.cs index 327f1c4810..3ab01da54f 100644 --- a/src/Microsoft.AspNetCore.Sockets.Client.Http/WebSocketsTransport.cs +++ b/src/Microsoft.AspNetCore.Sockets.Client.Http/WebSocketsTransport.cs @@ -81,7 +81,7 @@ private async Task ReceiveMessages(Uri pollUrl) try { - while (!_transportCts.Token.IsCancellationRequested) + while (!_receiveCts.Token.IsCancellationRequested) { const int bufferSize = 4096; var totalBytes = 0; @@ -130,15 +130,24 @@ private async Task ReceiveMessages(Uri pollUrl) Buffer.BlockCopy(incomingMessage[0].Array, incomingMessage[0].Offset, messageBuffer, 0, incomingMessage[0].Count); } - _logger.MessageToApp(_connectionId, messageBuffer.Length); - while (await _application.Out.WaitToWriteAsync(_transportCts.Token)) + try { - if (_application.Out.TryWrite(messageBuffer)) + if (!_transportCts.Token.IsCancellationRequested) { - incomingMessage.Clear(); - break; + _logger.MessageToApp(_connectionId, messageBuffer.Length); + while (await _application.Out.WaitToWriteAsync(_transportCts.Token)) + { + if (_application.Out.TryWrite(messageBuffer)) + { + incomingMessage.Clear(); + break; + } + } } } + catch (OperationCanceledException) + { + } } } catch (OperationCanceledException) From 7b2736c6a5af12c6c81a00be2d47cb2b981156c0 Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Fri, 18 Aug 2017 08:22:40 -0700 Subject: [PATCH 6/6] log --- .../Internal/SocketClientLoggerExtensions.cs | 11 +++++++++++ .../WebSocketsTransport.cs | 1 + 2 files changed, 12 insertions(+) diff --git a/src/Microsoft.AspNetCore.Sockets.Client.Http/Internal/SocketClientLoggerExtensions.cs b/src/Microsoft.AspNetCore.Sockets.Client.Http/Internal/SocketClientLoggerExtensions.cs index 4b277f1dc7..32d8bd52ed 100644 --- a/src/Microsoft.AspNetCore.Sockets.Client.Http/Internal/SocketClientLoggerExtensions.cs +++ b/src/Microsoft.AspNetCore.Sockets.Client.Http/Internal/SocketClientLoggerExtensions.cs @@ -62,6 +62,9 @@ internal static class SocketClientLoggerExtensions private static readonly Action _closingWebSocketFailed = LoggerMessage.Define(LogLevel.Information, 16, "{time}: Connection Id {connectionId}: Closing webSocket failed."); + private static readonly Action _cancelMessage = + LoggerMessage.Define(LogLevel.Debug, 17, "{time}: Connection Id {connectionId}: Canceled passing message to application."); + // Category: ServerSentEventsTransport and LongPollingTransport private static readonly Action _sendingMessages = LoggerMessage.Define(LogLevel.Debug, 9, "{time}: Connection Id {connectionId}: Sending {count} message(s) to the server using url: {url}."); @@ -283,6 +286,14 @@ public static void ClosingWebSocketFailed(this ILogger logger, string connection } } + public static void CancelMessage(this ILogger logger, string connectionId) + { + if (logger.IsEnabled(LogLevel.Debug)) + { + _cancelMessage(logger, DateTime.Now, connectionId, null); + } + } + public static void SendingMessages(this ILogger logger, string connectionId, int count, Uri url) { if (logger.IsEnabled(LogLevel.Debug)) diff --git a/src/Microsoft.AspNetCore.Sockets.Client.Http/WebSocketsTransport.cs b/src/Microsoft.AspNetCore.Sockets.Client.Http/WebSocketsTransport.cs index 3ab01da54f..2249c44bc9 100644 --- a/src/Microsoft.AspNetCore.Sockets.Client.Http/WebSocketsTransport.cs +++ b/src/Microsoft.AspNetCore.Sockets.Client.Http/WebSocketsTransport.cs @@ -147,6 +147,7 @@ private async Task ReceiveMessages(Uri pollUrl) } catch (OperationCanceledException) { + _logger.CancelMessage(_connectionId); } } }