From 16aa4b5775acc8273b3d9ae6105c1d4e74eeac91 Mon Sep 17 00:00:00 2001 From: Stevan Freeborn <65925598+StevanFreeborn@users.noreply.github.com> Date: Wed, 14 May 2025 16:45:44 -0500 Subject: [PATCH] fix: correct locking issue that was preventing progress --- .vscode/launch.json | 21 +++++ .vscode/settings.json | 7 ++ .vscode/tasks.json | 41 +++++++++ .../Unit/DiscordGatewayClientTests.cs | 91 +++++++++++-------- .../Discord/DiscordGatewayClient.cs | 31 +++++-- .../Discord/DiscordRestClient.cs | 2 +- .../Discord/DiscordRestClientException.cs | 2 +- .../Events/HeartbeatAckDiscordEvent.cs | 2 +- .../Discord/Events/HeartbeatDiscordEvent.cs | 2 +- 9 files changed, 147 insertions(+), 52 deletions(-) create mode 100644 .vscode/launch.json create mode 100644 .vscode/settings.json create mode 100644 .vscode/tasks.json diff --git a/.vscode/launch.json b/.vscode/launch.json new file mode 100644 index 0000000..532eb13 --- /dev/null +++ b/.vscode/launch.json @@ -0,0 +1,21 @@ +{ + "version": "0.2.0", + "configurations": [ + { + "name": "Worker", + "type": "coreclr", + "request": "launch", + "preLaunchTask": "build", + "program": "${workspaceFolder}/src/StevesBot.Worker/bin/Debug/net9.0/StevesBot.Worker.dll", + "args": [], + "cwd": "${workspaceFolder}/src/StevesBot.Worker", + "console": "internalConsole", + "stopAtEntry": false + }, + { + "name": ".NET Core Attach", + "type": "coreclr", + "request": "attach" + } + ] +} \ No newline at end of file diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000..f9c5899 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,7 @@ +{ + "editor.formatOnSave": true, + "dotnet.defaultSolution": "src/StevesBot.sln", + "cSpell.words": [ + "Steves" + ], +} \ No newline at end of file diff --git a/.vscode/tasks.json b/.vscode/tasks.json new file mode 100644 index 0000000..09d469a --- /dev/null +++ b/.vscode/tasks.json @@ -0,0 +1,41 @@ +{ + "version": "2.0.0", + "tasks": [ + { + "label": "build", + "command": "dotnet", + "type": "process", + "args": [ + "build", + "${workspaceFolder}/src/StevesBot.sln", + "/property:GenerateFullPaths=true", + "/consoleloggerparameters:NoSummary;ForceNoAlign" + ], + "problemMatcher": "$msCompile" + }, + { + "label": "publish", + "command": "dotnet", + "type": "process", + "args": [ + "publish", + "${workspaceFolder}/src/StevesBot.sln", + "/property:GenerateFullPaths=true", + "/consoleloggerparameters:NoSummary;ForceNoAlign" + ], + "problemMatcher": "$msCompile" + }, + { + "label": "watch", + "command": "dotnet", + "type": "process", + "args": [ + "watch", + "run", + "--project", + "${workspaceFolder}/src/StevesBot.sln" + ], + "problemMatcher": "$msCompile" + } + ] +} \ No newline at end of file diff --git a/src/StevesBot.Worker.Tests/Unit/DiscordGatewayClientTests.cs b/src/StevesBot.Worker.Tests/Unit/DiscordGatewayClientTests.cs index 195e34d..5fb39ef 100644 --- a/src/StevesBot.Worker.Tests/Unit/DiscordGatewayClientTests.cs +++ b/src/StevesBot.Worker.Tests/Unit/DiscordGatewayClientTests.cs @@ -107,67 +107,80 @@ public sealed class DiscordGatewayClientTests : IDisposable var messageQueue = new Queue<(WebSocketReceiveResult, byte[])>(); - var heatbeatInterval = 1000; + var heartbeatInterval = 1000; - var helloEvent = new + var helloEventPayload = CreateEventPayload(new { op = 10, d = new { - heartbeat_interval = heatbeatInterval, + heartbeat_interval = heartbeatInterval, } - }; + }); - var helloEventJson = JsonSerializer.Serialize(helloEvent); - var helloEventBytes = Encoding.UTF8.GetBytes(helloEventJson); + messageQueue.Enqueue(( + new(helloEventPayload.Bytes.Length, WebSocketMessageType.Text, true), + helloEventPayload.Bytes + )); - var heartbeatAck = new - { - op = 11, - }; - - var heartbeatAckJson = JsonSerializer.Serialize(heartbeatAck); - var heartbeatAckBytes = Encoding.UTF8.GetBytes(heartbeatAckJson); - - messageQueue.Enqueue((new WebSocketReceiveResult(helloEventBytes.Length, WebSocketMessageType.Text, true), helloEventBytes)); - messageQueue.Enqueue((new WebSocketReceiveResult(heartbeatAckBytes.Length, WebSocketMessageType.Text, true), heartbeatAckBytes)); - - mockWebSocket - .Setup(static x => x.SendAsync(It.IsAny>(), It.IsAny(), It.IsAny(), It.IsAny())) - .Returns(Task.CompletedTask); - - mockWebSocket - .Setup(static x => x.ReceiveAsync(It.IsAny>(), It.IsAny())) - .Returns((ArraySegment buffer, CancellationToken token) => - { - if (messageQueue.Count == 0) - { -# pragma warning disable CA2008 - return Task.Delay(-1, token).ContinueWith(_ => new WebSocketReceiveResult(0, WebSocketMessageType.Text, true), token); - } - - var (result, messageBytes) = messageQueue.Dequeue(); - Array.Copy(messageBytes, 0, buffer.Array!, buffer.Offset, Math.Min(messageBytes.Length, buffer.Count)); - return Task.FromResult(result); - }); + SetupReceiveMessageSequence(mockWebSocket, messageQueue); _mockWebSocketFactory .Setup(static x => x.Create()) .Returns(mockWebSocket.Object); - using var cts = new CancellationTokenSource(); + var cts = new CancellationTokenSource(); + await _discordGatewayClient.ConnectAsync(cts.Token); - await _discordGatewayClient.ConnectAsync(CancellationToken.None); + await Task.Delay((int)(heartbeatInterval * 1.5)); + await cts.CancelAsync(); - await Task.Delay(heatbeatInterval + 30000); + var expectedHeartbeatPayload = CreateEventPayload(new HeartbeatDiscordEvent(null)); mockWebSocket.Verify( - static x => x.SendAsync(It.IsAny>(), It.IsAny(), It.IsAny(), It.IsAny()), + x => x.SendAsync( + It.Is>(b => expectedHeartbeatPayload.Bytes.SequenceEqual(b)), + It.IsAny(), + It.IsAny(), + It.IsAny() + ), Times.Once ); } + private static void SetupReceiveMessageSequence( + Mock mockWebSocket, + Queue<(WebSocketReceiveResult, byte[])> messageQueue + ) + { + mockWebSocket + .Setup(static x => x.ReceiveAsync(It.IsAny>(), It.IsAny())) + .Returns((ArraySegment buffer, CancellationToken token) => + { + if (messageQueue.Count == 0) + { + return Task.Delay(-1, token) + .ContinueWith( + _ => new WebSocketReceiveResult(0, WebSocketMessageType.Text, true), + TaskScheduler.Default + ); + } + + var (result, messageBytes) = messageQueue.Dequeue(); + Array.Copy(messageBytes, 0, buffer.Array!, buffer.Offset, Math.Min(messageBytes.Length, buffer.Count)); + return Task.FromResult(result); + }); + } + + private static (byte[] Bytes, string Json) CreateEventPayload(object e) + { + var json = JsonSerializer.Serialize(e); + var bytes = Encoding.UTF8.GetBytes(json); + return (bytes, json); + } + public void Dispose() { + _discordGatewayClient.Dispose(); } } \ No newline at end of file diff --git a/src/StevesBot.Worker/Discord/DiscordGatewayClient.cs b/src/StevesBot.Worker/Discord/DiscordGatewayClient.cs index 3e0e267..cfec6d5 100644 --- a/src/StevesBot.Worker/Discord/DiscordGatewayClient.cs +++ b/src/StevesBot.Worker/Discord/DiscordGatewayClient.cs @@ -2,7 +2,7 @@ using System.Text; namespace StevesBot.Worker.Discord; -internal class DiscordGatewayClient : IDiscordGatewayClient +internal sealed class DiscordGatewayClient : IDiscordGatewayClient { private readonly DiscordClientOptions _options; private readonly IWebSocketFactory _webSocketFactory; @@ -80,6 +80,12 @@ internal class DiscordGatewayClient : IDiscordGatewayClient } } + if (_webSocket?.State is not WebSocketState.Open) + { + _logger.LogWarning("WebSocket is not open. Cannot receive messages."); + return; + } + // websocket message might be larger than the // size of the buffer so we need to loop until // we receive the end of the message and @@ -245,13 +251,10 @@ internal class DiscordGatewayClient : IDiscordGatewayClient private async Task SendJsonAsync(object data, CancellationToken cancellationToken) { - using (await _lock.LockAsync(cancellationToken)) + if (_webSocket?.State is not WebSocketState.Open) { - if (_webSocket?.State is not WebSocketState.Open) - { - _logger.LogWarning("WebSocket is not open. Cannot send message."); - return; - } + _logger.LogWarning("WebSocket is not open. Cannot send message."); + return; } try @@ -275,10 +278,20 @@ internal class DiscordGatewayClient : IDiscordGatewayClient public void Dispose() { _heartbeatCts?.Cancel(); + _linkedCts?.Cancel(); + + if (_heartbeatTask is not null && _heartbeatTask.IsCompleted) + { + _heartbeatTask.Dispose(); + } + + if (_receiveTask is not null && _receiveTask.IsCompleted) + { + _receiveTask.Dispose(); + } + _heartbeatCts?.Dispose(); _linkedCts?.Dispose(); - _heartbeatTask?.Dispose(); - _receiveTask?.Dispose(); _webSocket?.Dispose(); _lock.Dispose(); } diff --git a/src/StevesBot.Worker/Discord/DiscordRestClient.cs b/src/StevesBot.Worker/Discord/DiscordRestClient.cs index 7a9d651..76db332 100644 --- a/src/StevesBot.Worker/Discord/DiscordRestClient.cs +++ b/src/StevesBot.Worker/Discord/DiscordRestClient.cs @@ -1,6 +1,6 @@ namespace StevesBot.Worker.Discord; -internal class DiscordRestClient : IDiscordRestClient +internal sealed class DiscordRestClient : IDiscordRestClient { private readonly ILogger _logger; private readonly HttpClient _httpClient; diff --git a/src/StevesBot.Worker/Discord/DiscordRestClientException.cs b/src/StevesBot.Worker/Discord/DiscordRestClientException.cs index 583f8c1..70b5fb3 100644 --- a/src/StevesBot.Worker/Discord/DiscordRestClientException.cs +++ b/src/StevesBot.Worker/Discord/DiscordRestClientException.cs @@ -1,6 +1,6 @@ namespace StevesBot.Worker.Discord; -internal class DiscordRestClientException : Exception +internal sealed class DiscordRestClientException : Exception { public DiscordRestClientException() { diff --git a/src/StevesBot.Worker/Discord/Events/HeartbeatAckDiscordEvent.cs b/src/StevesBot.Worker/Discord/Events/HeartbeatAckDiscordEvent.cs index e67fde8..53cb764 100644 --- a/src/StevesBot.Worker/Discord/Events/HeartbeatAckDiscordEvent.cs +++ b/src/StevesBot.Worker/Discord/Events/HeartbeatAckDiscordEvent.cs @@ -1,5 +1,5 @@ namespace StevesBot.Worker.Discord.Events; -internal record HeartbeatAckDiscordEvent : DiscordEvent +internal sealed record HeartbeatAckDiscordEvent : DiscordEvent { } \ No newline at end of file diff --git a/src/StevesBot.Worker/Discord/Events/HeartbeatDiscordEvent.cs b/src/StevesBot.Worker/Discord/Events/HeartbeatDiscordEvent.cs index a0b6aaa..874d652 100644 --- a/src/StevesBot.Worker/Discord/Events/HeartbeatDiscordEvent.cs +++ b/src/StevesBot.Worker/Discord/Events/HeartbeatDiscordEvent.cs @@ -1,6 +1,6 @@ namespace StevesBot.Worker.Discord.Events; -internal record HeartbeatDiscordEvent : DiscordEvent +internal sealed record HeartbeatDiscordEvent : DiscordEvent { public HeartbeatDiscordEvent(int? sequence) {