From ac389f5f6823e3a720aedd81b7805adbdd78b66d Mon Sep 17 00:00:00 2001 From: Christopher Felegy Date: Sat, 22 Dec 2018 12:41:34 -0500 Subject: [PATCH] fix: DefaultWebSocket can now correctly close This resolves a bug where disconnecting the socket client would not actually close the websocket. Bots would appear to remain online in the discord client until their connection to discord eventually timed out. The underlying cause of this issue sourced from the cancellation token passed into the websocket's ReceiveAsync method - when entering the disconnect process, the first step is to cancel out all of the connection tokens. Unfortunately, the standard ClientWebSocket handles a token cancellation by aborting the socket, rendering it inoperable for a safe closure. This change removes the inner cancellation token passed into ReceiveAsync. The cancellation token is still retained for use in the receive loop, so the receive task should gracefully complete once some event satisfies the ClientWebSocket's blocking receive. To ensure that all clients succesfully close, regardless of their traffic, the disconnect procedure was rearranged such that awaiting the receive task now occurs last, after the socket has been closed. Closing the socket will propagate an event up to the ClientWebSocket's receive method, which will allow the loop to iterate and gracefully complete. So far, I have validated this change against basic connection opening and closing, for both the gateway and voice clients. I have not yet validated against unplanned connection interruptions, though I believe that this change might actually improve some of those connection bugs, since the ClientWebSocket should never find itself in an aborted state. --- .../Net/DefaultWebSocketClient.cs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/Discord.Net.WebSocket/Net/DefaultWebSocketClient.cs b/src/Discord.Net.WebSocket/Net/DefaultWebSocketClient.cs index df2da5813..36a6fea4f 100644 --- a/src/Discord.Net.WebSocket/Net/DefaultWebSocketClient.cs +++ b/src/Discord.Net.WebSocket/Net/DefaultWebSocketClient.cs @@ -108,15 +108,10 @@ namespace Discord.Net.WebSockets } private async Task DisconnectInternalAsync(bool isDisposing = false) { - try { _disconnectTokenSource.Cancel(false); } catch { } + try { _disconnectTokenSource.Cancel(false); } + catch { } _isDisconnecting = true; - try - { - await (_task ?? Task.Delay(0)).ConfigureAwait(false); - _task = null; - } - finally { _isDisconnecting = false; } if (_client != null) { @@ -130,6 +125,13 @@ namespace Discord.Net.WebSockets _client = null; } + + try + { + await (_task ?? Task.Delay(0)).ConfigureAwait(false); + _task = null; + } + finally { _isDisconnecting = false; } } private async Task OnClosed(Exception ex) { @@ -198,7 +200,7 @@ namespace Discord.Net.WebSockets { while (!cancelToken.IsCancellationRequested) { - WebSocketReceiveResult socketResult = await _client.ReceiveAsync(buffer, cancelToken).ConfigureAwait(false); + WebSocketReceiveResult socketResult = await _client.ReceiveAsync(buffer, CancellationToken.None).ConfigureAwait(false); byte[] result; int resultCount;