diff --git a/MinecraftClient.Tests/AutoRelogRetryPolicyTests.cs b/MinecraftClient.Tests/AutoRelogRetryPolicyTests.cs new file mode 100644 index 00000000..18416d09 --- /dev/null +++ b/MinecraftClient.Tests/AutoRelogRetryPolicyTests.cs @@ -0,0 +1,145 @@ +using MinecraftClient.ChatBots; +using MinecraftClient.Scripting; + +namespace MinecraftClient.Tests; + +public sealed class AutoRelogRetryPolicyTests +{ + [Fact] + public void DefaultConfigurationUsesUnlimitedRetries() + { + Assert.Equal(-1, new AutoRelog.Configs().Retries); + } + + [Fact] + public void UnlimitedRetriesNeverExhaust() + { + var policy = new AutoRelogRetryPolicy(new ManualTimeProvider()); + + for (int attempt = 1; attempt <= 100; attempt++) + { + Assert.True(policy.TryReserveAttempt(-1, out int retriesLeft)); + Assert.Equal(-1, retriesLeft); + Assert.Equal(attempt, policy.Attempts); + } + } + + [Fact] + public void ZeroRetriesDisablesReconnect() + { + var policy = new AutoRelogRetryPolicy(new ManualTimeProvider()); + + Assert.False(policy.TryReserveAttempt(0, out int retriesLeft)); + Assert.Equal(0, retriesLeft); + Assert.Equal(0, policy.Attempts); + } + + [Fact] + public void FiniteRetryLimitIsExact() + { + var policy = new AutoRelogRetryPolicy(new ManualTimeProvider()); + + Assert.True(policy.TryReserveAttempt(3, out int firstRetriesLeft)); + Assert.True(policy.TryReserveAttempt(3, out int secondRetriesLeft)); + Assert.True(policy.TryReserveAttempt(3, out int thirdRetriesLeft)); + Assert.False(policy.TryReserveAttempt(3, out int exhaustedRetriesLeft)); + + Assert.Equal(2, firstRetriesLeft); + Assert.Equal(1, secondRetriesLeft); + Assert.Equal(0, thirdRetriesLeft); + Assert.Equal(0, exhaustedRetriesLeft); + } + + [Fact] + public void RejectedRestartDoesNotConsumeRetry() + { + var policy = new AutoRelogRetryPolicy(new ManualTimeProvider()); + + Assert.True(policy.TryReserveAttempt(1, out _)); + policy.RollBackReservedAttempt(); + + Assert.True(policy.TryReserveAttempt(1, out int retriesLeft)); + Assert.Equal(0, retriesLeft); + } + + [Fact] + public void StableConnectionResetsRetryBudget() + { + var timeProvider = new ManualTimeProvider(); + var policy = new AutoRelogRetryPolicy(timeProvider); + Assert.True(policy.TryReserveAttempt(1, out _)); + + policy.MarkJoined(); + timeProvider.Advance(AutoRelogRetryPolicy.StableConnectionThreshold - TimeSpan.FromMilliseconds(1)); + Assert.False(policy.ResetAfterStableConnection()); + + timeProvider.Advance(TimeSpan.FromMilliseconds(1)); + Assert.True(policy.ResetAfterStableConnection()); + Assert.Equal(0, policy.Attempts); + Assert.True(policy.TryReserveAttempt(1, out _)); + } + + [Fact] + public void TransportLossAlwaysReconnectsWhenEnabled() + { + bool reconnect = AutoRelogRetryPolicy.ShouldReconnect( + ChatBot.DisconnectReason.ConnectionLost, + "A transport-specific error without a configured phrase", + ignoreKickMessage: false, + ["Server is restarting"], + out string? matchedMessage); + + Assert.True(reconnect); + Assert.Null(matchedMessage); + } + + [Theory] + [InlineData(ChatBot.DisconnectReason.InGameKick)] + [InlineData(ChatBot.DisconnectReason.LoginRejected)] + public void ServerMessageMatchingIsCaseInsensitive(ChatBot.DisconnectReason reason) + { + bool reconnect = AutoRelogRetryPolicy.ShouldReconnect( + reason, + "THE SERVER IS RESTARTING NOW", + ignoreKickMessage: false, + ["server is restarting"], + out string? matchedMessage); + + Assert.True(reconnect); + Assert.Equal("server is restarting", matchedMessage); + } + + [Fact] + public void UserLogoutNeverReconnects() + { + Assert.False(AutoRelogRetryPolicy.ShouldReconnect( + ChatBot.DisconnectReason.UserLogout, + "Server is restarting", + ignoreKickMessage: true, + ["Server is restarting"], + out _)); + } + + [Fact] + public void IgnoreKickMessageAllowsNonmatchingServerKick() + { + Assert.True(AutoRelogRetryPolicy.ShouldReconnect( + ChatBot.DisconnectReason.InGameKick, + "Administrative removal", + ignoreKickMessage: true, + [], + out _)); + } + + private sealed class ManualTimeProvider : TimeProvider + { + private DateTimeOffset utcNow = DateTimeOffset.UnixEpoch; + + public override DateTimeOffset GetUtcNow() => utcNow; + + internal void Advance(TimeSpan duration) + { + utcNow += duration; + } + } +} diff --git a/MinecraftClient.Tests/MinecraftClient.Tests.csproj b/MinecraftClient.Tests/MinecraftClient.Tests.csproj new file mode 100644 index 00000000..84440e31 --- /dev/null +++ b/MinecraftClient.Tests/MinecraftClient.Tests.csproj @@ -0,0 +1,22 @@ + + + net10.0 + enable + enable + false + true + + + + + + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + + + + + + + diff --git a/MinecraftClient.Tests/RestartCoordinatorTests.cs b/MinecraftClient.Tests/RestartCoordinatorTests.cs new file mode 100644 index 00000000..41e7d81b --- /dev/null +++ b/MinecraftClient.Tests/RestartCoordinatorTests.cs @@ -0,0 +1,61 @@ +namespace MinecraftClient.Tests; + +public sealed class RestartCoordinatorTests +{ + [Fact] + public async Task CoalescesSameAttemptAndQueuesNewerAttempt() + { + var firstStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var releaseFirst = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var secondCompleted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + using var coordinator = new RestartCoordinator( + async (request, cancellationToken) => + { + if (request.ConnectionAttempt == 10) + { + firstStarted.SetResult(); + await releaseFirst.Task.WaitAsync(cancellationToken); + } + else if (request.ConnectionAttempt == 11) + { + secondCompleted.SetResult(); + } + }, + exception => throw new Xunit.Sdk.XunitException(exception.ToString())); + + Assert.True(coordinator.TrySchedule(new RestartRequest(10, TimeSpan.Zero, true))); + await firstStarted.Task.WaitAsync(TimeSpan.FromSeconds(5)); + + Assert.False(coordinator.TrySchedule(new RestartRequest(10, TimeSpan.Zero, true))); + Assert.True(coordinator.TrySchedule(new RestartRequest(11, TimeSpan.Zero, true))); + Assert.True(coordinator.HasScheduledRestart(11)); + + releaseFirst.SetResult(); + await secondCompleted.Task.WaitAsync(TimeSpan.FromSeconds(5)); + } + + [Fact] + public void RejectsStaleAttempt() + { + using var coordinator = new RestartCoordinator( + (_, _) => Task.CompletedTask, + exception => throw new Xunit.Sdk.XunitException(exception.ToString())); + + Assert.True(coordinator.TrySchedule(new RestartRequest(20, TimeSpan.Zero, true))); + Assert.False(coordinator.TrySchedule(new RestartRequest(19, TimeSpan.Zero, true))); + } + + [Fact] + public void TerminalStopRejectsFurtherRestarts() + { + using var coordinator = new RestartCoordinator( + (_, _) => Task.CompletedTask, + exception => throw new Xunit.Sdk.XunitException(exception.ToString())); + + coordinator.Stop(); + + Assert.False(coordinator.TrySchedule(new RestartRequest(1, TimeSpan.Zero, true))); + Assert.False(coordinator.HasScheduledRestart(1)); + } +} diff --git a/MinecraftClient.Tests/SocketWrapperTests.cs b/MinecraftClient.Tests/SocketWrapperTests.cs new file mode 100644 index 00000000..c04fb9b4 --- /dev/null +++ b/MinecraftClient.Tests/SocketWrapperTests.cs @@ -0,0 +1,34 @@ +using System.Net; +using System.Net.Sockets; +using MinecraftClient.Protocol.Handlers; + +namespace MinecraftClient.Tests; + +public sealed class SocketWrapperTests +{ + [Fact] + public async Task GracefulPeerCloseEndsReadInsteadOfSpinning() + { + var listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + try + { + using var client = new TcpClient(); + Task acceptTask = listener.AcceptTcpClientAsync(); + await client.ConnectAsync((IPEndPoint)listener.LocalEndpoint); + using TcpClient peer = await acceptTask; + var wrapper = new SocketWrapper(client); + + peer.Client.Shutdown(SocketShutdown.Both); + peer.Close(); + + Assert.True(SpinWait.SpinUntil(wrapper.HasDataAvailable, TimeSpan.FromSeconds(5))); + await Assert.ThrowsAsync( + () => Task.Run(() => wrapper.ReadDataRAW(1)).WaitAsync(TimeSpan.FromSeconds(5))); + } + finally + { + listener.Stop(); + } + } +} diff --git a/MinecraftClient.Tests/Usings.cs b/MinecraftClient.Tests/Usings.cs new file mode 100644 index 00000000..c802f448 --- /dev/null +++ b/MinecraftClient.Tests/Usings.cs @@ -0,0 +1 @@ +global using Xunit; diff --git a/MinecraftClient.sln b/MinecraftClient.sln index ebdf1f09..3f24b221 100644 --- a/MinecraftClient.sln +++ b/MinecraftClient.sln @@ -13,6 +13,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "MccMcpStdioHarness", "Debug EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "MccMcpWebPlayground", "DebugTools\MccMcpWebPlayground\MccMcpWebPlayground.csproj", "{5F620CF6-BC7D-449A-B779-2D51985059C6}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "MinecraftClient.Tests", "MinecraftClient.Tests\MinecraftClient.Tests.csproj", "{44B63F7B-30E2-47DA-B2C8-A8742BC8AE0B}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -71,6 +73,18 @@ Global {5F620CF6-BC7D-449A-B779-2D51985059C6}.Release|x64.Build.0 = Release|Any CPU {5F620CF6-BC7D-449A-B779-2D51985059C6}.Release|x86.ActiveCfg = Release|Any CPU {5F620CF6-BC7D-449A-B779-2D51985059C6}.Release|x86.Build.0 = Release|Any CPU + {44B63F7B-30E2-47DA-B2C8-A8742BC8AE0B}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {44B63F7B-30E2-47DA-B2C8-A8742BC8AE0B}.Debug|Any CPU.Build.0 = Debug|Any CPU + {44B63F7B-30E2-47DA-B2C8-A8742BC8AE0B}.Debug|x64.ActiveCfg = Debug|Any CPU + {44B63F7B-30E2-47DA-B2C8-A8742BC8AE0B}.Debug|x64.Build.0 = Debug|Any CPU + {44B63F7B-30E2-47DA-B2C8-A8742BC8AE0B}.Debug|x86.ActiveCfg = Debug|Any CPU + {44B63F7B-30E2-47DA-B2C8-A8742BC8AE0B}.Debug|x86.Build.0 = Debug|Any CPU + {44B63F7B-30E2-47DA-B2C8-A8742BC8AE0B}.Release|Any CPU.ActiveCfg = Release|Any CPU + {44B63F7B-30E2-47DA-B2C8-A8742BC8AE0B}.Release|Any CPU.Build.0 = Release|Any CPU + {44B63F7B-30E2-47DA-B2C8-A8742BC8AE0B}.Release|x64.ActiveCfg = Release|Any CPU + {44B63F7B-30E2-47DA-B2C8-A8742BC8AE0B}.Release|x64.Build.0 = Release|Any CPU + {44B63F7B-30E2-47DA-B2C8-A8742BC8AE0B}.Release|x86.ActiveCfg = Release|Any CPU + {44B63F7B-30E2-47DA-B2C8-A8742BC8AE0B}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/MinecraftClient/ChatBots/AutoRelog.cs b/MinecraftClient/ChatBots/AutoRelog.cs index 1c975b38..af3b3e71 100644 --- a/MinecraftClient/ChatBots/AutoRelog.cs +++ b/MinecraftClient/ChatBots/AutoRelog.cs @@ -1,5 +1,4 @@ using System; -using System.Threading; using MinecraftClient.Scripting; using Tomlet.Attributes; @@ -24,7 +23,7 @@ namespace MinecraftClient.ChatBots public Range Delay = new(3); [TomlInlineComment("$ChatBot.AutoRelog.Retries$")] - public int Retries = 3; + public int Retries = -1; [TomlInlineComment("$ChatBot.AutoRelog.Ignore_Kick_Message$")] public bool Ignore_Kick_Message = false; @@ -32,27 +31,27 @@ namespace MinecraftClient.ChatBots [TomlPrecedingComment("$ChatBot.AutoRelog.Kick_Messages$")] public string[] Kick_Messages = new string[] { "Connection has been lost", "Server is restarting", "Server is full", "Too Many people" }; - [NonSerialized] - public static int _BotRecoAttempts = 0; - public void OnSettingUpdate() { + Kick_Messages ??= Array.Empty(); + + if (!double.IsFinite(Delay.min)) + Delay.min = 0.1; + if (!double.IsFinite(Delay.max)) + Delay.max = 0.1; + Delay.min = Math.Max(0.1, Delay.min); Delay.max = Math.Max(0.1, Delay.max); - double maxDelaySeconds = int.MaxValue / (double)Settings.ClientTicksPerSecond; + double maxDelaySeconds = (uint.MaxValue - 1) / 1000D; Delay.min = Math.Min(maxDelaySeconds, Delay.min); Delay.max = Math.Min(maxDelaySeconds, Delay.max); if (Delay.min > Delay.max) (Delay.min, Delay.max) = (Delay.max, Delay.min); - if (Retries == -1) - Retries = int.MaxValue; - - if (Enabled) - for (int i = 0; i < Kick_Messages.Length; i++) - Kick_Messages[i] = Kick_Messages[i].ToLower(); + if (Retries < -1) + Retries = -1; } public struct Range @@ -78,9 +77,7 @@ namespace MinecraftClient.ChatBots } } - private static readonly Lock s_reconnectStateLock = new(); - private static readonly TimeSpan s_stableJoinBeforeRetryReset = TimeSpan.FromSeconds(60); - private static DateTime? s_lastJoinUtc; + private static readonly AutoRelogRetryPolicy s_retryPolicy = new(TimeProvider.System); /// /// This bot automatically re-join the server if kick message contains predefined string @@ -100,8 +97,7 @@ namespace MinecraftClient.ChatBots public override void AfterGameJoined() { - lock (s_reconnectStateLock) - s_lastJoinUtc = DateTime.UtcNow; + s_retryPolicy.MarkJoined(); } public override void Update() @@ -123,96 +119,40 @@ namespace MinecraftClient.ChatBots if (reason == DisconnectReason.UserLogout) { LogDebugToConsole(Translations.bot_autoRelog_ignore_user_logout); + return false; } - else if (Program.HasRestartPendingForAnotherThread) + + message = GetVerbatim(message); + LogDebugToConsole(string.Format(Translations.bot_autoRelog_disconnect_msg, message)); + + if (!AutoRelogRetryPolicy.ShouldReconnect( + reason, + message, + Config.Ignore_Kick_Message, + Config.Kick_Messages, + out string? matchedMessage)) { - return true; - } - else if (CanReconnect()) - { - message = GetVerbatim(message); - string comp = message.ToLower(); - - LogDebugToConsole(string.Format(Translations.bot_autoRelog_disconnect_msg, message)); - - if (Config.Ignore_Kick_Message) - { - return LaunchDelayedReconnection(null); - } - - foreach (string msg in Config.Kick_Messages) - { - if (comp.Contains(msg)) - { - return LaunchDelayedReconnection(msg); - } - } - LogDebugToConsole(Translations.bot_autoRelog_reconnect_ignore); + return false; } - return false; - } - - private static bool CanReconnect() - { - lock (s_reconnectStateLock) - return Config.Retries < 0 || Configs._BotRecoAttempts < Config.Retries; + return LaunchDelayedReconnection(matchedMessage); } private static void ResetRetriesAfterStableJoin() { - lock (s_reconnectStateLock) - { - if (Configs._BotRecoAttempts <= 0 || s_lastJoinUtc is not DateTime lastJoinUtc) - return; - - if (DateTime.UtcNow - lastJoinUtc < s_stableJoinBeforeRetryReset) - return; - - Configs._BotRecoAttempts = 0; - s_lastJoinUtc = null; + if (s_retryPolicy.ResetAfterStableConnection()) McClient.ReconnectionAttemptsLeft = Config.Retries; - } - } - - private static bool TryConsumeReconnectAttempt(out int retriesLeft) - { - lock (s_reconnectStateLock) - { - bool unlimitedRetries = HasUnlimitedRetries(); - if (!unlimitedRetries && Configs._BotRecoAttempts >= Config.Retries) - { - retriesLeft = 0; - return false; - } - - Configs._BotRecoAttempts++; - s_lastJoinUtc = null; - retriesLeft = unlimitedRetries ? int.MaxValue : Config.Retries - Configs._BotRecoAttempts; - if (retriesLeft < 0) - retriesLeft = 0; - return true; - } } private static bool HasUnlimitedRetries() { - return Config.Retries < 0 || Config.Retries == int.MaxValue; - } - - private static void RollBackReconnectAttempt() - { - lock (s_reconnectStateLock) - { - if (Configs._BotRecoAttempts > 0) - Configs._BotRecoAttempts--; - } + return Config.Retries == -1; } private bool LaunchDelayedReconnection(string? msg) { - if (!TryConsumeReconnectAttempt(out int retriesLeft)) + if (!s_retryPolicy.TryReserveAttempt(Config.Retries, out int retriesLeft)) return false; double delay = Random.Shared.NextDouble() * (Config.Delay.max - Config.Delay.min) + Config.Delay.min; @@ -223,14 +163,14 @@ namespace MinecraftClient.ChatBots : retriesLeft.ToString(); McClient.ReconnectionAttemptsLeft = retriesLeft; - if (Program.TryRestart((int)Math.Floor(delay), true)) + if (Program.TryRestart(TimeSpan.FromSeconds(delay), true)) { LogToConsole(string.Format(Translations.bot_autoRelog_wait_with_retries, delay, retriesDisplay)); return true; } - RollBackReconnectAttempt(); - return true; + s_retryPolicy.RollBackReservedAttempt(); + return Program.HasRestartPending; } public static bool OnDisconnectStatic(DisconnectReason reason, string message) diff --git a/MinecraftClient/ChatBots/AutoRelogRetryPolicy.cs b/MinecraftClient/ChatBots/AutoRelogRetryPolicy.cs new file mode 100644 index 00000000..259b7079 --- /dev/null +++ b/MinecraftClient/ChatBots/AutoRelogRetryPolicy.cs @@ -0,0 +1,108 @@ +using System; +using System.Threading; +using MinecraftClient.Scripting; + +namespace MinecraftClient.ChatBots +{ + internal sealed class AutoRelogRetryPolicy + { + internal static readonly TimeSpan StableConnectionThreshold = TimeSpan.FromSeconds(60); + + private readonly Lock stateLock = new(); + private readonly TimeProvider timeProvider; + private int attempts; + private DateTimeOffset? joinedAt; + + internal AutoRelogRetryPolicy(TimeProvider timeProvider) + { + ArgumentNullException.ThrowIfNull(timeProvider); + this.timeProvider = timeProvider; + } + + internal int Attempts + { + get + { + lock (stateLock) + return attempts; + } + } + + internal void MarkJoined() + { + lock (stateLock) + joinedAt = timeProvider.GetUtcNow(); + } + + internal bool ResetAfterStableConnection() + { + lock (stateLock) + { + if (attempts == 0 || joinedAt is not DateTimeOffset connectionStart) + return false; + + if (timeProvider.GetUtcNow() - connectionStart < StableConnectionThreshold) + return false; + + attempts = 0; + joinedAt = null; + return true; + } + } + + internal bool TryReserveAttempt(int retryLimit, out int retriesLeft) + { + lock (stateLock) + { + bool unlimited = retryLimit == -1; + if (!unlimited && attempts >= retryLimit) + { + retriesLeft = 0; + return false; + } + + attempts++; + joinedAt = null; + retriesLeft = unlimited ? -1 : Math.Max(0, retryLimit - attempts); + return true; + } + } + + internal void RollBackReservedAttempt() + { + lock (stateLock) + { + if (attempts > 0) + attempts--; + } + } + + internal static bool ShouldReconnect( + ChatBot.DisconnectReason reason, + string message, + bool ignoreKickMessage, + ReadOnlySpan kickMessages, + out string? matchedMessage) + { + matchedMessage = null; + + if (reason == ChatBot.DisconnectReason.UserLogout) + return false; + + if (reason == ChatBot.DisconnectReason.ConnectionLost || ignoreKickMessage) + return true; + + foreach (string candidate in kickMessages) + { + if (!string.IsNullOrEmpty(candidate) + && message.Contains(candidate, StringComparison.OrdinalIgnoreCase)) + { + matchedMessage = candidate; + return true; + } + } + + return false; + } + } +} diff --git a/MinecraftClient/ConsoleIO.cs b/MinecraftClient/ConsoleIO.cs index 2c5a168c..ee1072e3 100644 --- a/MinecraftClient/ConsoleIO.cs +++ b/MinecraftClient/ConsoleIO.cs @@ -77,6 +77,22 @@ namespace MinecraftClient /// public static string? ReadPassword() { + if (ConsoleInputRouter.IsStarted) + { + if (BasicIO || Backend is null) + return ConsoleInputRouter.ReadLine(); + + Backend.SetInputVisible(false); + try + { + return ConsoleInputRouter.ReadLine(); + } + finally + { + Backend.SetInputVisible(true); + } + } + if (BasicIO) return Console.ReadLine(); return Backend.ReadPassword(); @@ -87,6 +103,9 @@ namespace MinecraftClient /// public static string ReadLine() { + if (ConsoleInputRouter.IsStarted) + return ConsoleInputRouter.ReadLine(); + if (BasicIO) return Console.ReadLine() ?? String.Empty; return Backend.RequestImmediateInput(); diff --git a/MinecraftClient/ConsoleInputRouter.cs b/MinecraftClient/ConsoleInputRouter.cs new file mode 100644 index 00000000..3343e4b2 --- /dev/null +++ b/MinecraftClient/ConsoleInputRouter.cs @@ -0,0 +1,187 @@ +using System; +using System.Threading; +using System.Threading.Tasks; + +namespace MinecraftClient +{ + internal static class ConsoleInputRouter + { + private static readonly Lock StateLock = new(); + private static readonly CancellationTokenSource Shutdown = new(); + private static bool started; + private static Action? messageRoute; + private static EventHandler? inputChangeRoute; + private static TaskCompletionSource? pendingRead; + + internal static bool IsStarted + { + get + { + lock (StateLock) + return started; + } + } + + internal static void EnsureStarted() + { + lock (StateLock) + { + if (started) + return; + + started = true; + if (ConsoleIO.BasicIO || ConsoleIO.Backend is null) + { + var readThread = new Thread(() => ReadBasicInput(Shutdown.Token)) + { + Name = "MCC console input router", + }; + readThread.Start(); + return; + } + + try + { + ConsoleIO.Backend.MessageReceived += OnMessageReceived; + ConsoleIO.Backend.OnInputChange += OnInputChanged; + ConsoleIO.Backend.BeginReadThread(); + } + catch + { + ConsoleIO.Backend.MessageReceived -= OnMessageReceived; + ConsoleIO.Backend.OnInputChange -= OnInputChanged; + started = false; + throw; + } + } + } + + internal static void RouteToClient(McClient client) + { + ArgumentNullException.ThrowIfNull(client); + EnsureStarted(); + + lock (StateLock) + { + messageRoute = client.RouteConsoleInput; + inputChangeRoute = ConsoleIO.AutocompleteHandler; + } + } + + internal static void ClearClient(McClient client) + { + ArgumentNullException.ThrowIfNull(client); + + lock (StateLock) + { + if (messageRoute == client.RouteConsoleInput) + { + messageRoute = null; + inputChangeRoute = null; + } + } + } + + internal static void RouteOffline(Action handler) + { + ArgumentNullException.ThrowIfNull(handler); + EnsureStarted(); + + lock (StateLock) + { + messageRoute = handler; + inputChangeRoute = ConsoleIO.OfflineAutocompleteHandler; + } + } + + internal static void ClearOfflineRoute(Action handler) + { + ArgumentNullException.ThrowIfNull(handler); + + lock (StateLock) + { + if (messageRoute == handler) + { + messageRoute = null; + inputChangeRoute = null; + } + } + } + + internal static string ReadLine() + { + EnsureStarted(); + + TaskCompletionSource readCompletion = new(TaskCreationOptions.RunContinuationsAsynchronously); + lock (StateLock) + { + if (pendingRead is not null) + throw new InvalidOperationException(); + + pendingRead = readCompletion; + } + + return readCompletion.Task.GetAwaiter().GetResult(); + } + + internal static void ShutdownRouter() + { + lock (StateLock) + { + messageRoute = null; + inputChangeRoute = null; + pendingRead?.TrySetCanceled(); + pendingRead = null; + Shutdown.Cancel(); + } + } + + private static void ReadBasicInput(CancellationToken cancellationToken) + { + while (!cancellationToken.IsCancellationRequested) + { + string? input = Console.ReadLine(); + if (input is null) + return; + + DispatchMessage(input); + } + } + + private static void OnMessageReceived(object? sender, string message) + { + DispatchMessage(message); + } + + private static void OnInputChanged(object? sender, ConsoleInputBuffer input) + { + EventHandler? route; + lock (StateLock) + route = inputChangeRoute; + + route?.Invoke(sender, input); + } + + private static void DispatchMessage(string message) + { + TaskCompletionSource? readCompletion; + Action? route; + + lock (StateLock) + { + readCompletion = pendingRead; + if (readCompletion is not null) + pendingRead = null; + route = messageRoute; + } + + if (readCompletion is not null) + { + readCompletion.TrySetResult(message); + return; + } + + route?.Invoke(message); + } + } +} diff --git a/MinecraftClient/McClient.cs b/MinecraftClient/McClient.cs index 82dfa741..c20b03dd 100644 --- a/MinecraftClient/McClient.cs +++ b/MinecraftClient/McClient.cs @@ -5,6 +5,7 @@ using System.Net; using System.Net.Sockets; using System.Text; using System.Threading; +using System.Threading.Tasks; using Brigadier.NET; using Brigadier.NET.Exceptions; using MinecraftClient.ChatBots; @@ -228,12 +229,11 @@ namespace MinecraftClient TcpClient client = null!; IMinecraftCom handler = null!; SessionToken _sessionToken; - CancellationTokenSource? cmdprompt = null; Tuple? timeoutdetector = null; - private Thread? basicIOReadThread; private int transferInProgress = 0; - private bool consoleReadThreadOwned = false; - private bool consoleHandlersAttached = false; + private int disconnectState; + private int disconnectOwnerThreadId; + private readonly TaskCompletionSource disconnectCompletion = new(TaskCreationOptions.RunContinuationsAsynchronously); public ILogger Log; public DialogManager Dialogs { get; } @@ -530,75 +530,12 @@ namespace MinecraftClient private void StartConsoleSession() { - cmdprompt = new CancellationTokenSource(); - - if (ConsoleIO.BasicIO || ConsoleIO.Backend is null) - { - if (!consoleReadThreadOwned) - { - CancellationToken token = cmdprompt.Token; - basicIOReadThread = new Thread(() => BasicIOReadLoop(token)) - { - IsBackground = true, - Name = "MCC BasicIO read thread" - }; - basicIOReadThread.Start(); - consoleReadThreadOwned = true; - } - - return; - } - - if (!consoleReadThreadOwned) - { - ConsoleIO.Backend.BeginReadThread(); - consoleReadThreadOwned = true; - } - - if (!consoleHandlersAttached) - { - ConsoleIO.Backend.MessageReceived += ConsoleReaderOnMessageReceived; - ConsoleIO.Backend.OnInputChange += ConsoleIO.AutocompleteHandler; - consoleHandlersAttached = true; - } + ConsoleInputRouter.RouteToClient(this); } private void StopConsoleSession() { - if (ConsoleIO.BasicIO || ConsoleIO.Backend is null) - { - cmdprompt?.Cancel(); - basicIOReadThread = null; - consoleReadThreadOwned = false; - consoleHandlersAttached = false; - return; - } - - if (consoleHandlersAttached) - { - ConsoleIO.Backend.MessageReceived -= ConsoleReaderOnMessageReceived; - ConsoleIO.Backend.OnInputChange -= ConsoleIO.AutocompleteHandler; - consoleHandlersAttached = false; - } - - if (consoleReadThreadOwned) - { - ConsoleIO.Backend.StopReadThread(); - consoleReadThreadOwned = false; - } - } - - private void BasicIOReadLoop(CancellationToken token) - { - while (!token.IsCancellationRequested) - { - string? input = Console.ReadLine(); - if (input is null) - return; - - if (!token.IsCancellationRequested) - ConsoleReaderOnMessageReceived(this, input); - } + ConsoleInputRouter.ClearClient(this); } private void ResetStateForTransfer() @@ -866,36 +803,21 @@ namespace MinecraftClient /// public void Disconnect() { - instance = null; - - DispatchBotEvent(bot => bot.OnDisconnect(ChatBot.DisconnectReason.UserLogout, "")); - - foreach (ChatBot bot in bots.Where(bot => bot.ScriptOwnerKey is not null).ToList()) - BotUnLoad(bot); - - botsOnHold.Clear(); - botsOnHold.AddRange(bots.Where(bot => bot.ScriptOwnerKey is null)); - - if (handler is not null) + if (!TryBeginDisconnect()) { - handler.Disconnect(); - handler.Dispose(); + if (Volatile.Read(ref disconnectOwnerThreadId) != Environment.CurrentManagedThreadId) + disconnectCompletion.Task.GetAwaiter().GetResult(); + return; } - if (cmdprompt is not null) + try { - cmdprompt.Cancel(); - cmdprompt = null; + DispatchBotEvent(bot => bot.OnDisconnect(ChatBot.DisconnectReason.UserLogout, string.Empty)); } - - if (timeoutdetector is not null) + finally { - timeoutdetector.Item2.Cancel(); - timeoutdetector = null; + CompleteDisconnect(sendDisconnectPacket: true); } - - if (client is not null) - client.Close(); } /// @@ -903,74 +825,133 @@ namespace MinecraftClient /// public void OnConnectionLost(ChatBot.DisconnectReason reason, string message) { - instance = null; + if (reason == ChatBot.DisconnectReason.UserLogout) + throw new InvalidOperationException(Translations.exception_user_logout); - ConsoleIO.CancelAutocomplete(); + if (!TryBeginDisconnect()) + return; - handler.Dispose(); - - world.Clear(); - ClearKnownSigns(); - - if (timeoutdetector is not null) + bool restartScheduled = false; + try { - if (timeoutdetector is not null && Thread.CurrentThread != timeoutdetector.Item1) - timeoutdetector.Item2.Cancel(); - timeoutdetector = null; - } + ConsoleIO.CancelAutocomplete(); - bool exitOnFailure = Program.PrepareExitOnFailure(); - bool will_restart = false; + world.Clear(); + ClearKnownSigns(); - switch (reason) - { - case ChatBot.DisconnectReason.ConnectionLost: - message = Translations.mcc_disconnect_lost; - Log.Info(message); - break; + bool exitOnFailure = Program.PrepareExitOnFailure(); - case ChatBot.DisconnectReason.InGameKick: - Log.Info(Translations.mcc_disconnect_server); - Log.Info(message); - break; - - case ChatBot.DisconnectReason.LoginRejected: - Log.Info(Translations.mcc_disconnect_login); - Log.Info(message); - break; - - case ChatBot.DisconnectReason.UserLogout: - throw new InvalidOperationException(Translations.exception_user_logout); - } - - //Process AutoRelog last to make sure other bots can perform their cleanup tasks first (issue #1517) - List onDisconnectBotList = bots.Where(bot => bot is not AutoRelog).ToList(); - onDisconnectBotList.AddRange(bots.Where(bot => bot is AutoRelog)); - - foreach (ChatBot bot in onDisconnectBotList) - { - try + switch (reason) { - bool botWillRestart = bot.OnDisconnect(reason, message); - if (!exitOnFailure) - will_restart |= botWillRestart; + case ChatBot.DisconnectReason.ConnectionLost: + message = Translations.mcc_disconnect_lost; + Log.Info(message); + break; + + case ChatBot.DisconnectReason.InGameKick: + Log.Info(Translations.mcc_disconnect_server); + Log.Info(message); + break; + + case ChatBot.DisconnectReason.LoginRejected: + Log.Info(Translations.mcc_disconnect_login); + Log.Info(message); + break; } - catch (Exception e) + + // Process AutoRelog last so every other bot can complete cleanup first. + List onDisconnectBotList = bots.Where(bot => bot is not AutoRelog).ToList(); + onDisconnectBotList.AddRange(bots.Where(bot => bot is AutoRelog)); + + foreach (ChatBot bot in onDisconnectBotList) { - if (e is not ThreadAbortException) + try { - Log.Warn("OnDisconnect: Got error from " + bot.ToString() + ": " + e.ToString()); + _ = bot.OnDisconnect(reason, message); + } + catch (Exception exception) when (exception is not ThreadAbortException) + { + Log.Warn("OnDisconnect: Got error from " + bot + ": " + exception); } - else throw; //ThreadAbortException should not be caught } + + restartScheduled = !exitOnFailure && Program.HasRestartPending; + } + finally + { + CompleteDisconnect(sendDisconnectPacket: false); } - SentrySdk.EndSession(); - - if (!will_restart) - { - StopConsoleSession(); + if (!restartScheduled) Program.HandleFailure(null, false, reason); + } + + private bool TryBeginDisconnect() + { + if (Interlocked.CompareExchange(ref disconnectState, 1, 0) != 0) + return false; + + Volatile.Write(ref disconnectOwnerThreadId, Environment.CurrentManagedThreadId); + instance = null; + StopConsoleSession(); + return true; + } + + private void CompleteDisconnect(bool sendDisconnectPacket) + { + try + { + foreach (ChatBot bot in bots.Where(bot => bot.ScriptOwnerKey is not null).ToList()) + { + try + { + BotUnLoad(bot); + } + catch (Exception exception) when (exception is not ThreadAbortException) + { + Log.Warn(exception.ToString()); + } + } + + botsOnHold.Clear(); + botsOnHold.AddRange(bots.Where(bot => bot.ScriptOwnerKey is null)); + + if (timeoutdetector is not null) + { + CancellationTokenSource timeoutCancellation = timeoutdetector.Item2; + timeoutdetector = null; + RunDisconnectCleanup(timeoutCancellation.Cancel); + } + + if (handler is not null) + { + if (sendDisconnectPacket) + RunDisconnectCleanup(handler.Disconnect); + RunDisconnectCleanup(handler.Dispose); + } + + if (client is not null) + RunDisconnectCleanup(client.Close); + ClearTasks(); + RunDisconnectCleanup(() => SentrySdk.EndSession()); + } + finally + { + Volatile.Write(ref disconnectState, 2); + Volatile.Write(ref disconnectOwnerThreadId, 0); + disconnectCompletion.TrySetResult(true); + } + } + + private void RunDisconnectCleanup(Action cleanup) + { + try + { + cleanup(); + } + catch (Exception exception) when (exception is not ThreadAbortException) + { + Log.Warn(exception.ToString()); } } @@ -980,19 +961,16 @@ namespace MinecraftClient private void ConsoleReaderOnMessageReceived(object? sender, string e) { - if (client.Client is null) return; if (client.Client.Connected) - { - new Thread(() => - { - InvokeOnMainThread(() => HandleCommandPromptText(e)); - }).Start(); - } - else - return; + InvokeOnMainThreadAsync(() => HandleCommandPromptText(e)); + } + + internal void RouteConsoleInput(string input) + { + ConsoleReaderOnMessageReceived(this, input); } /// @@ -1221,6 +1199,23 @@ namespace MinecraftClient InvokeOnMainThread(() => { task(); return true; }); } + /// + /// Queue work for the network main thread without blocking the calling thread. + /// + internal void InvokeOnMainThreadAsync(Action task) + { + ArgumentNullException.ThrowIfNull(task); + + if (!InvokeRequired) + { + task(); + return; + } + + lock (threadTasksLock) + threadTasks.Enqueue(task); + } + /// /// Clear all tasks /// diff --git a/MinecraftClient/Program.cs b/MinecraftClient/Program.cs index 5408f6b7..35d6f1fb 100644 --- a/MinecraftClient/Program.cs +++ b/MinecraftClient/Program.cs @@ -51,11 +51,11 @@ namespace MinecraftClient public const string MCHighestVersion = "26.2"; public static readonly string? BuildInfo = null; - private static Tuple? offlinePrompt = null; private static IDisposable? _sentrySdk = null; private static bool useMcVersionOnce = false; - private static Thread? _restartThread = null; - private static readonly object _restartLock = new(); + private static readonly RestartCoordinator restartCoordinator = new(ExecuteRestartAsync, ReportRestartFailure); + private static long connectionAttempt; + private static int offlinePromptActive; private static int exitOnFailurePending; private static string settingsIniPath = "MinecraftClient.ini"; @@ -608,6 +608,8 @@ namespace MinecraftClient /// private static void InitializeClient() { + Interlocked.Increment(ref connectionAttempt); + // Ensure that we use the provided Minecraft version if we can't connect automatically. // // useMcVersionOnce is set to true on HandleFailure() @@ -737,6 +739,8 @@ namespace MinecraftClient Config.Main.SetServerIP(new MainConfigHelper.MainConfig.ServerInfoConfig(addressInput), true); } + ConsoleInputRouter.EnsureStarted(); + //Get server version int protocolversion = 0; ForgeInfo? forgeInfo = null; @@ -901,69 +905,60 @@ namespace MinecraftClient /// Optional, keep account and server settings public static void Restart(int delaySeconds = 0, bool keepAccountAndServerSettings = false) { - TryRestart(delaySeconds, keepAccountAndServerSettings); + TryRestart(TimeSpan.FromSeconds(Math.Max(0, delaySeconds)), keepAccountAndServerSettings); } - internal static bool HasRestartPendingForAnotherThread - { - get - { - lock (_restartLock) - return HasRestartPendingForAnotherThreadNoLock(); - } - } + internal static bool HasRestartPending => restartCoordinator.HasScheduledRestart(Volatile.Read(ref connectionAttempt)); internal static bool TryRestart(int delaySeconds = 0, bool keepAccountAndServerSettings = false) { - lock (_restartLock) - { - if (Volatile.Read(ref exitOnFailurePending) != 0) - return false; - - if (HasRestartPendingForAnotherThreadNoLock()) - return false; - - ConsoleIO.Backend?.StopReadThread(); - var thread = new Thread(new ThreadStart(delegate - { - try - { - if (client is not null) { client.Disconnect(); ConsoleIO.Reset(); } - if (offlinePrompt is not null) - { - if (ConsoleIO.Backend is not null) - ConsoleIO.Backend.OnInputChange -= ConsoleIO.OfflineAutocompleteHandler; - offlinePrompt.Item2.Cancel(); offlinePrompt.Item1.Join(); offlinePrompt = null; ConsoleIO.Reset(); - } - if (delaySeconds > 0) - { - ConsoleIO.WriteLine(string.Format(Translations.mcc_restart_delay, delaySeconds)); - Thread.Sleep(delaySeconds * 1000); - } - ConsoleIO.WriteLine(Translations.mcc_restart); - ReloadSettings(keepAccountAndServerSettings); - InitializeClient(); - } - finally - { - lock (_restartLock) - { - if (_restartThread == Thread.CurrentThread) - _restartThread = null; - } - } - })); - _restartThread = thread; - thread.Start(); - return true; - } + return TryRestart(TimeSpan.FromSeconds(Math.Max(0, delaySeconds)), keepAccountAndServerSettings); } - private static bool HasRestartPendingForAnotherThreadNoLock() + internal static bool TryRestart(TimeSpan delay, bool keepAccountAndServerSettings = false) { - return _restartThread is not null - && _restartThread.IsAlive - && _restartThread != Thread.CurrentThread; + if (Volatile.Read(ref exitOnFailurePending) != 0) + return false; + + if (delay < TimeSpan.Zero) + delay = TimeSpan.Zero; + + return restartCoordinator.TrySchedule(new RestartRequest( + Volatile.Read(ref connectionAttempt), + delay, + keepAccountAndServerSettings)); + } + + private static async Task ExecuteRestartAsync(RestartRequest request, CancellationToken cancellationToken) + { + McClient? disconnectedClient = client; + if (disconnectedClient is not null) + { + disconnectedClient.Disconnect(); + if (ReferenceEquals(client, disconnectedClient)) + client = null; + } + + EndOfflinePrompt(); + ConsoleIO.Reset(); + + if (request.Delay > TimeSpan.Zero) + { + ConsoleIO.WriteLine(string.Format(Translations.mcc_restart_delay, request.Delay.TotalSeconds)); + await Task.Delay(request.Delay, TimeProvider.System, cancellationToken).ConfigureAwait(false); + } + + cancellationToken.ThrowIfCancellationRequested(); + ConsoleIO.WriteLine(Translations.mcc_restart); + ReloadSettings(request.KeepAccountAndServerSettings); + InitializeClient(); + } + + private static void ReportRestartFailure(Exception exception) + { + SentrySdk.CaptureException(exception); + ConsoleIO.WriteLine(exception.ToString()); + HandleFailure(); } /// @@ -976,6 +971,7 @@ namespace MinecraftClient return false; Interlocked.Exchange(ref exitOnFailurePending, 1); + restartCoordinator.Stop(); return true; } @@ -984,17 +980,10 @@ namespace MinecraftClient WriteBackSettings(); ConsoleIO.WriteLineFormatted("§a" + string.Format(Translations.config_saving, settingsIniPath)); + restartCoordinator.Stop(); if (client is not null) { client.Disconnect(); ConsoleIO.Reset(); } - if (offlinePrompt is not null) - { - if (ConsoleIO.Backend is not null) - ConsoleIO.Backend.OnInputChange -= ConsoleIO.OfflineAutocompleteHandler; - offlinePrompt.Item2.Cancel(); - if (Thread.CurrentThread != offlinePrompt.Item1) - offlinePrompt.Item1.Join(1000); - offlinePrompt = null; - ConsoleIO.Reset(); - } + EndOfflinePrompt(); + ConsoleInputRouter.ShutdownRouter(); if (Config.Main.Advanced.PlayerHeadAsIcon && OperatingSystem.IsWindows()) { ConsoleIcon.RevertToMCCIcon(); } ConsoleIO.Backend?.Shutdown(); Environment.Exit(exitcode); @@ -1022,7 +1011,7 @@ namespace MinecraftClient if (!string.IsNullOrEmpty(errorMessage)) { ConsoleIO.Reset(); - if (ConsoleIO.Backend is not Tui.TuiConsoleBackend) + if (!ConsoleInputRouter.IsStarted && ConsoleIO.Backend is not Tui.TuiConsoleBackend) { try { @@ -1067,85 +1056,89 @@ namespace MinecraftClient return; } - if (offlinePrompt is null) + BeginOfflinePrompt(); + } + } + + private static void BeginOfflinePrompt() + { + if (Interlocked.CompareExchange(ref offlinePromptActive, 1, 0) != 0) + return; + + ConsoleInputRouter.RouteOffline(HandleOfflineCommand); + ConsoleIO.WriteLine(string.Empty); + ConsoleIO.WriteLineFormatted(string.Format(Translations.mcc_disconnected, Config.Main.Advanced.InternalCmdChar.ToLogString())); + if (ConsoleIO.Backend is Tui.TuiConsoleBackend) + ConsoleIO.WriteLineFormatted(string.Format(Translations.mcc_use_quit_to_exit, Config.Main.Advanced.InternalCmdChar.ToLogString())); + else + ConsoleIO.WriteLineFormatted(Translations.mcc_press_exit, acceptnewlines: true); + } + + private static void EndOfflinePrompt() + { + if (Interlocked.Exchange(ref offlinePromptActive, 0) == 0) + return; + + ConsoleInputRouter.ClearOfflineRoute(HandleOfflineCommand); + ConsoleIO.Reset(); + } + + private static void HandleOfflineCommand(string input) + { + string command = input.Trim(); + if (command.Length == 0) + { + if (ConsoleIO.Backend is not Tui.TuiConsoleBackend) + Commands.Exit.DoExit(Config.AppVar.ExpandVars(command)); + return; + } + + if (Config.Main.Advanced.InternalCmdChar.ToChar() != ' ' + && command[0] == Config.Main.Advanced.InternalCmdChar.ToChar()) + { + command = command[1..]; + } + + string message = string.Empty; + if (command.StartsWith("reco", StringComparison.Ordinal)) + { + message = Commands.Reco.DoReconnect(Config.AppVar.ExpandVars(command)); + if (message.Length == 0) { - ConsoleIO.Backend?.StopReadThread(); - if (ConsoleIO.Backend is not null) - ConsoleIO.Backend.OnInputChange += ConsoleIO.OfflineAutocompleteHandler; - - var cancellationTokenSource = new CancellationTokenSource(); - offlinePrompt = new(new Thread(new ThreadStart(delegate - { - bool exitThread = false; - string command = " "; - ConsoleIO.WriteLine(string.Empty); - ConsoleIO.WriteLineFormatted(string.Format(Translations.mcc_disconnected, Config.Main.Advanced.InternalCmdChar.ToLogString())); - if (ConsoleIO.Backend is Tui.TuiConsoleBackend) - ConsoleIO.WriteLineFormatted(string.Format(Translations.mcc_use_quit_to_exit, Config.Main.Advanced.InternalCmdChar.ToLogString())); - else - ConsoleIO.WriteLineFormatted(Translations.mcc_press_exit, acceptnewlines: true); - - while (!cancellationTokenSource.IsCancellationRequested) - { - if (exitThread) - return; - - command = ConsoleIO.ReadLine().Trim(); - - if (command.Length == 0) - { - if (ConsoleIO.Backend is not Tui.TuiConsoleBackend) - Commands.Exit.DoExit(Config.AppVar.ExpandVars(command)); - continue; - } - - string message = ""; - - if (Config.Main.Advanced.InternalCmdChar.ToChar() != ' ' - && command[0] == Config.Main.Advanced.InternalCmdChar.ToChar()) - command = command[1..]; - - if (command.StartsWith("reco")) - { - message = Commands.Reco.DoReconnect(Config.AppVar.ExpandVars(command)); - if (message == "") - { - exitThread = true; - continue; - } - } - else if (command.StartsWith("connect")) - { - message = Commands.Connect.DoConnect(Config.AppVar.ExpandVars(command)); - if (message == "") - { - exitThread = true; - continue; - } - } - else if (command.StartsWith("exit") || command.StartsWith("quit")) - { - message = Commands.Exit.DoExit(Config.AppVar.ExpandVars(command)); - } - else if (command.StartsWith("help")) - { - ConsoleIO.WriteLineFormatted("§8MCC: " + - Config.Main.Advanced.InternalCmdChar.ToLogString() + - new Commands.Reco().GetCmdDescTranslated()); - ConsoleIO.WriteLineFormatted("§8MCC: " + - Config.Main.Advanced.InternalCmdChar.ToLogString() + - new Commands.Connect().GetCmdDescTranslated()); - } - else - ConsoleIO.WriteLineFormatted(string.Format(Translations.icmd_unknown, command.Split(' ')[0])); - - if (message != "") - ConsoleIO.WriteLineFormatted("§8MCC: " + message); - } - })), cancellationTokenSource); - offlinePrompt.Item1.Start(); + EndOfflinePrompt(); + return; } } + else if (command.StartsWith("connect", StringComparison.Ordinal)) + { + message = Commands.Connect.DoConnect(Config.AppVar.ExpandVars(command)); + if (message.Length == 0) + { + EndOfflinePrompt(); + return; + } + } + else if (command.StartsWith("exit", StringComparison.Ordinal) + || command.StartsWith("quit", StringComparison.Ordinal)) + { + message = Commands.Exit.DoExit(Config.AppVar.ExpandVars(command)); + } + else if (command.StartsWith("help", StringComparison.Ordinal)) + { + ConsoleIO.WriteLineFormatted("§8MCC: " + + Config.Main.Advanced.InternalCmdChar.ToLogString() + + new Commands.Reco().GetCmdDescTranslated()); + ConsoleIO.WriteLineFormatted("§8MCC: " + + Config.Main.Advanced.InternalCmdChar.ToLogString() + + new Commands.Connect().GetCmdDescTranslated()); + } + else + { + ConsoleIO.WriteLineFormatted(string.Format(Translations.icmd_unknown, command.Split(' ')[0])); + } + + if (message.Length != 0) + ConsoleIO.WriteLineFormatted("§8MCC: " + message); } private static int GetFailureExitCode(ChatBot.DisconnectReason? disconnectReason) diff --git a/MinecraftClient/Properties/AssemblyInfo.cs b/MinecraftClient/Properties/AssemblyInfo.cs index 99b66052..7e608e53 100644 --- a/MinecraftClient/Properties/AssemblyInfo.cs +++ b/MinecraftClient/Properties/AssemblyInfo.cs @@ -1,6 +1,9 @@ using System.Reflection; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices; +[assembly: InternalsVisibleTo("MinecraftClient.Tests")] + // General Information about an assembly is controlled through the following // set of attributes. Change these attribute values to modify the information // associated with an assembly. diff --git a/MinecraftClient/Protocol/Handlers/Protocol18.cs b/MinecraftClient/Protocol/Handlers/Protocol18.cs index 2d0bb40f..968e6465 100644 --- a/MinecraftClient/Protocol/Handlers/Protocol18.cs +++ b/MinecraftClient/Protocol/Handlers/Protocol18.cs @@ -4082,20 +4082,19 @@ namespace MinecraftClient.Protocol.Handlers { try { - if (netMain is not null) + netMain?.Item2.Cancel(); + } + finally + { + try { - netMain.Item2.Cancel(); + netReader?.Item2.Cancel(); } - - if (netReader is not null) + finally { - netReader.Item2.Cancel(); socketWrapper.Disconnect(); } } - catch - { - } } /// diff --git a/MinecraftClient/Protocol/Handlers/SocketWrapper.cs b/MinecraftClient/Protocol/Handlers/SocketWrapper.cs index a4f451b1..7aaac71c 100644 --- a/MinecraftClient/Protocol/Handlers/SocketWrapper.cs +++ b/MinecraftClient/Protocol/Handlers/SocketWrapper.cs @@ -1,4 +1,5 @@ using System; +using System.IO; using System.Net.Sockets; using MinecraftClient.Crypto; @@ -38,7 +39,7 @@ namespace MinecraftClient.Protocol.Handlers /// TRUE if data is available to read public bool HasDataAvailable() { - return c.Client.Available > 0; + return c.Client.Available > 0 || c.Client.Poll(0, SelectMode.SelectRead); } /// @@ -61,10 +62,16 @@ namespace MinecraftClient.Protocol.Handlers int read = 0; while (read < offset) { + int bytesRead; if (encrypted) - read += s!.Read(buffer, start + read, offset - read); + bytesRead = s!.Read(buffer, start + read, offset - read); else - read += c.Client.Receive(buffer, start + read, offset - read, f); + bytesRead = c.Client.Receive(buffer, start + read, offset - read, f); + + if (bytesRead == 0) + throw new EndOfStreamException(); + + read += bytesRead; } } diff --git a/MinecraftClient/Resources/ConfigComments/ConfigComments.Designer.cs b/MinecraftClient/Resources/ConfigComments/ConfigComments.Designer.cs index cdd5d4b1..41c1eee0 100644 --- a/MinecraftClient/Resources/ConfigComments/ConfigComments.Designer.cs +++ b/MinecraftClient/Resources/ConfigComments/ConfigComments.Designer.cs @@ -663,7 +663,7 @@ namespace MinecraftClient { } /// - /// Looks up a localized string similar to When set to true, autorelog will reconnect regardless of kick messages.. + /// Looks up a localized string similar to Reconnect after any server kick or login rejection. Network interruptions always trigger Auto Relog.. /// internal static string ChatBot_AutoRelog_Ignore_Kick_Message { get { @@ -672,7 +672,7 @@ namespace MinecraftClient { } /// - /// Looks up a localized string similar to If the kickout message matches any of the strings, then autorelog will be triggered.. + /// Looks up a localized string similar to Case-insensitive text fragments that trigger Auto Relog for server kicks and login rejections.. /// internal static string ChatBot_AutoRelog_Kick_Messages { get { @@ -681,7 +681,7 @@ namespace MinecraftClient { } /// - /// Looks up a localized string similar to Retries when failing to relog to the server. use -1 for unlimited retries.. + /// Looks up a localized string similar to Exact retry limit. Use 0 to disable retries or -1 for unlimited retries. The count resets after 60 seconds online.. /// internal static string ChatBot_AutoRelog_Retries { get { diff --git a/MinecraftClient/Resources/ConfigComments/ConfigComments.resx b/MinecraftClient/Resources/ConfigComments/ConfigComments.resx index d1ebbcb2..42e3ee0c 100644 --- a/MinecraftClient/Resources/ConfigComments/ConfigComments.resx +++ b/MinecraftClient/Resources/ConfigComments/ConfigComments.resx @@ -352,13 +352,13 @@ You can use "/fish" to control the bot manually. The delay time before joining the server. (in seconds) - When set to true, autorelog will reconnect regardless of kick messages. + Reconnect after any server kick or login rejection. Network interruptions always trigger Auto Relog. - If the kickout message matches any of the strings, then autorelog will be triggered. + Case-insensitive text fragments that trigger Auto Relog for server kicks and login rejections. - Retries when failing to relog to the server. use -1 for unlimited retries. + Exact retry limit. Use 0 to disable retries or -1 for unlimited retries. The count resets after 60 seconds online. Run commands or send messages automatically when a specified pattern is detected in chat diff --git a/MinecraftClient/RestartCoordinator.cs b/MinecraftClient/RestartCoordinator.cs new file mode 100644 index 00000000..3193d933 --- /dev/null +++ b/MinecraftClient/RestartCoordinator.cs @@ -0,0 +1,119 @@ +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Channels; +using System.Threading.Tasks; + +namespace MinecraftClient +{ + internal readonly record struct RestartRequest( + long ConnectionAttempt, + TimeSpan Delay, + bool KeepAccountAndServerSettings); + + internal sealed class RestartCoordinator : IDisposable + { + private readonly Lock stateLock = new(); + private readonly Channel requests; + private readonly CancellationTokenSource shutdown = new(); + private readonly Func restart; + private readonly Action reportFailure; + private readonly Task worker; + private readonly HashSet pendingAttempts = []; + private long highestScheduledAttempt = -1; + private bool stopped; + + internal RestartCoordinator( + Func restart, + Action reportFailure) + { + ArgumentNullException.ThrowIfNull(restart); + ArgumentNullException.ThrowIfNull(reportFailure); + + this.restart = restart; + this.reportFailure = reportFailure; + requests = Channel.CreateUnbounded(new UnboundedChannelOptions + { + SingleReader = true, + SingleWriter = false, + AllowSynchronousContinuations = false, + }); + worker = ProcessRequestsAsync(); + } + + internal bool HasScheduledRestart(long connectionAttempt) + { + lock (stateLock) + return !stopped && pendingAttempts.Contains(connectionAttempt); + } + + internal bool TrySchedule(RestartRequest request) + { + lock (stateLock) + { + if (stopped || request.ConnectionAttempt <= highestScheduledAttempt) + return false; + + highestScheduledAttempt = request.ConnectionAttempt; + pendingAttempts.Add(request.ConnectionAttempt); + if (requests.Writer.TryWrite(request)) + return true; + + pendingAttempts.Remove(request.ConnectionAttempt); + return false; + } + } + + internal void Stop() + { + lock (stateLock) + { + if (stopped) + return; + + stopped = true; + pendingAttempts.Clear(); + requests.Writer.TryComplete(); + shutdown.Cancel(); + } + } + + private async Task ProcessRequestsAsync() + { + try + { + await foreach (RestartRequest request in requests.Reader.ReadAllAsync(shutdown.Token).ConfigureAwait(false)) + { + try + { + await restart(request, shutdown.Token).ConfigureAwait(false); + } + catch (OperationCanceledException) when (shutdown.IsCancellationRequested) + { + return; + } + catch (Exception exception) + { + reportFailure(exception); + } + finally + { + lock (stateLock) + pendingAttempts.Remove(request.ConnectionAttempt); + } + } + } + catch (OperationCanceledException) when (shutdown.IsCancellationRequested) + { + } + } + + public void Dispose() + { + Stop(); + worker.GetAwaiter().GetResult(); + shutdown.Dispose(); + GC.SuppressFinalize(this); + } + } +} diff --git a/docs/guide/chat-bots.md b/docs/guide/chat-bots.md index 23658d54..d93eb210 100644 --- a/docs/guide/chat-bots.md +++ b/docs/guide/chat-bots.md @@ -1318,7 +1318,9 @@ redirectFrom: - **Description:** - Make MCC automatically relog when disconnected by the server, for example because the server is restating. + Make MCC reconnect after a network interruption or a matching server kick. + + A lost TCP connection always triggers Auto Relog when the bot is enabled. `Kick_Messages` only filters server kick and login rejection messages. Logging out with an MCC command never triggers Auto Relog. - **Settings:** @@ -1343,9 +1345,9 @@ redirectFrom: - **Description:** - The delay time before joining the server. + The delay before the next connection attempt. - If the `min` and `max` are the same, the time will be consistent, however, if you want a random time, you can set `min` and `max` to different values to get a random time. The time format is in seconds, and the type is double. (eg. `37.0`) + If `min` and `max` are equal, every attempt uses that delay. Otherwise, MCC picks a random value in the range. Values are seconds and may include a fractional part, such as `0.5` or `37.0`. - **Format:** `{ min = , max = }` @@ -1365,9 +1367,9 @@ redirectFrom: - **Description:** - Number of retries. + Number of connection attempts after a disconnect. `0` disables retries, and a positive value is used as an exact limit. - Use `-1` for infinite retries. + Use `-1` for unlimited retries. MCC resets the count after the connection has remained stable for 60 seconds. A restart request that MCC rejects as a duplicate does not consume an attempt. - **Default:** `-1` @@ -1375,7 +1377,7 @@ redirectFrom: - **Description:** - This settings specifies if the `Kick_Messages` setting will be ignored, if set to `true` it will auto relog regardless of the kick messages. + Reconnect after any server kick or login rejection instead of checking `Kick_Messages`. This setting does not affect network interruptions, which always trigger Auto Relog. - **Type:** `boolean` @@ -1385,7 +1387,7 @@ redirectFrom: - **Description:** - A list of words which should trigger the Auto Reconnect Chat Bot. + Text fragments that trigger Auto Relog for server kicks and login rejections. Matching is case-insensitive. - **Format:** `[ "", "", ... ]` diff --git a/tools/testing/auto_relog_fault_proxy.py b/tools/testing/auto_relog_fault_proxy.py new file mode 100755 index 00000000..63e58249 --- /dev/null +++ b/tools/testing/auto_relog_fault_proxy.py @@ -0,0 +1,179 @@ +#!/usr/bin/env python3 +"""TCP proxy for repeatable Auto Relog connection-loss tests.""" + +from __future__ import annotations + +import argparse +import asyncio +import socket +import struct +import time +from contextlib import suppress + + +class FaultProxy: + def __init__( + self, + upstream_host: str, + upstream_port: int, + drop_after: float, + outage_seconds: float, + cycles: int, + reset_connection: bool, + ) -> None: + self.upstream_host = upstream_host + self.upstream_port = upstream_port + self.drop_after = drop_after + self.outage_seconds = outage_seconds + self.remaining_cycles = cycles + self.reset_connection = reset_connection + self.outage_until = 0.0 + self.state_lock = asyncio.Lock() + + async def handle_connection( + self, + client_reader: asyncio.StreamReader, + client_writer: asyncio.StreamWriter, + ) -> None: + peer = client_writer.get_extra_info("peername") + if time.monotonic() < self.outage_until: + print(f"reject peer={peer} reason=outage", flush=True) + await self.close_writer(client_writer) + return + + try: + server_reader, server_writer = await asyncio.open_connection( + self.upstream_host, + self.upstream_port, + ) + except OSError as exception: + print(f"reject peer={peer} reason=upstream error={exception}", flush=True) + await self.close_writer(client_writer) + return + + async with self.state_lock: + inject_fault = self.remaining_cycles != 0 + if self.remaining_cycles > 0: + self.remaining_cycles -= 1 + + print(f"connected peer={peer} inject_fault={inject_fault}", flush=True) + drop_task = ( + asyncio.create_task(self.drop_connection(client_writer, server_writer)) + if inject_fault + else None + ) + + relays = [ + asyncio.create_task(self.relay(client_reader, server_writer)), + asyncio.create_task(self.relay(server_reader, client_writer)), + ] + try: + await asyncio.wait(relays, return_when=asyncio.FIRST_COMPLETED) + finally: + for task in relays: + task.cancel() + if drop_task is not None: + drop_task.cancel() + await asyncio.gather(*relays, return_exceptions=True) + if drop_task is not None: + await asyncio.gather(drop_task, return_exceptions=True) + await self.close_writer(client_writer) + await self.close_writer(server_writer) + + async def drop_connection( + self, + client_writer: asyncio.StreamWriter, + server_writer: asyncio.StreamWriter, + ) -> None: + await asyncio.sleep(self.drop_after) + async with self.state_lock: + self.outage_until = max( + self.outage_until, + time.monotonic() + self.outage_seconds, + ) + + mode = "reset" if self.reset_connection else "graceful" + print(f"drop mode={mode} outage_seconds={self.outage_seconds}", flush=True) + if self.reset_connection: + self.set_reset_on_close(client_writer) + self.set_reset_on_close(server_writer) + client_writer.close() + server_writer.close() + + @staticmethod + async def relay( + reader: asyncio.StreamReader, + writer: asyncio.StreamWriter, + ) -> None: + while data := await reader.read(64 * 1024): + writer.write(data) + await writer.drain() + + @staticmethod + def set_reset_on_close(writer: asyncio.StreamWriter) -> None: + raw_socket = writer.get_extra_info("socket") + if raw_socket is not None: + raw_socket.setsockopt( + socket.SOL_SOCKET, + socket.SO_LINGER, + struct.pack("ii", 1, 0), + ) + + @staticmethod + async def close_writer(writer: asyncio.StreamWriter) -> None: + writer.close() + with suppress(ConnectionError, OSError): + await writer.wait_closed() + + +def parse_args() -> argparse.Namespace: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--listen-host", default="127.0.0.1") + parser.add_argument("--listen-port", type=int, required=True) + parser.add_argument("--upstream-host", default="127.0.0.1") + parser.add_argument("--upstream-port", type=int, required=True) + parser.add_argument("--drop-after", type=float, default=5.0) + parser.add_argument("--outage-seconds", type=float, default=10.0) + parser.add_argument( + "--cycles", + type=int, + default=1, + help="Connections to drop. Use -1 to drop every forwarded connection.", + ) + parser.add_argument( + "--mode", + choices=("graceful", "reset"), + default="graceful", + ) + args = parser.parse_args() + if args.drop_after < 0 or args.outage_seconds < 0 or args.cycles < -1: + parser.error("drop and outage values must be nonnegative; cycles must be -1 or greater") + return args + + +async def main() -> None: + args = parse_args() + proxy = FaultProxy( + args.upstream_host, + args.upstream_port, + args.drop_after, + args.outage_seconds, + args.cycles, + args.mode == "reset", + ) + server = await asyncio.start_server( + proxy.handle_connection, + args.listen_host, + args.listen_port, + ) + addresses = ", ".join(str(sock.getsockname()) for sock in server.sockets or []) + print(f"listening addresses={addresses}", flush=True) + async with server: + await server.serve_forever() + + +if __name__ == "__main__": + try: + asyncio.run(main()) + except KeyboardInterrupt: + pass