diff --git a/MinecraftClient.Tests/RestartCoordinatorTests.cs b/MinecraftClient.Tests/RestartCoordinatorTests.cs index 41e7d81b..80b41a3c 100644 --- a/MinecraftClient.Tests/RestartCoordinatorTests.cs +++ b/MinecraftClient.Tests/RestartCoordinatorTests.cs @@ -3,20 +3,27 @@ namespace MinecraftClient.Tests; public sealed class RestartCoordinatorTests { [Fact] - public async Task CoalescesSameAttemptAndQueuesNewerAttempt() + public async Task ReplacesQueuedSameAttemptAndQueuesNewerAttempt() { var firstStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var releaseFirst = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var replacementCompleted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var secondCompleted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var executedAccounts = new List(); using var coordinator = new RestartCoordinator( async (request, cancellationToken) => { - if (request.ConnectionAttempt == 10) + if (request.ConnectionAttempt == 9) { firstStarted.SetResult(); await releaseFirst.Task.WaitAsync(cancellationToken); } + else if (request.ConnectionAttempt == 10) + { + executedAccounts.Add(request.SettingsSnapshot?.Account.Login ?? string.Empty); + replacementCompleted.SetResult(); + } else if (request.ConnectionAttempt == 11) { secondCompleted.SetResult(); @@ -24,15 +31,19 @@ public sealed class RestartCoordinatorTests }, exception => throw new Xunit.Sdk.XunitException(exception.ToString())); - Assert.True(coordinator.TrySchedule(new RestartRequest(10, TimeSpan.Zero, true))); + Assert.True(coordinator.TrySchedule(new RestartRequest(9, 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(10, TimeSpan.Zero, true, CreateSettingsSnapshot("first")))); + Assert.True(coordinator.TrySchedule(new RestartRequest(10, TimeSpan.Zero, true, CreateSettingsSnapshot("replacement")))); Assert.True(coordinator.TrySchedule(new RestartRequest(11, TimeSpan.Zero, true))); Assert.True(coordinator.HasScheduledRestart(11)); releaseFirst.SetResult(); + await replacementCompleted.Task.WaitAsync(TimeSpan.FromSeconds(5)); await secondCompleted.Task.WaitAsync(TimeSpan.FromSeconds(5)); + + Assert.Equal(["replacement"], executedAccounts); } [Fact] @@ -46,6 +57,25 @@ public sealed class RestartCoordinatorTests Assert.False(coordinator.TrySchedule(new RestartRequest(19, TimeSpan.Zero, true))); } + [Fact] + public async Task RejectsCompletedAttempt() + { + var completed = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + using var coordinator = new RestartCoordinator( + (_, _) => + { + completed.SetResult(); + return Task.CompletedTask; + }, + exception => throw new Xunit.Sdk.XunitException(exception.ToString())); + + Assert.True(coordinator.TrySchedule(new RestartRequest(20, TimeSpan.Zero, true))); + await completed.Task.WaitAsync(TimeSpan.FromSeconds(5)); + Assert.True(SpinWait.SpinUntil(() => !coordinator.HasScheduledRestart(20), TimeSpan.FromSeconds(5))); + + Assert.False(coordinator.TrySchedule(new RestartRequest(20, TimeSpan.Zero, true))); + } + [Fact] public void TerminalStopRejectsFurtherRestarts() { @@ -58,4 +88,12 @@ public sealed class RestartCoordinatorTests Assert.False(coordinator.TrySchedule(new RestartRequest(1, TimeSpan.Zero, true))); Assert.False(coordinator.HasScheduledRestart(1)); } + + private static RestartSettingsSnapshot CreateSettingsSnapshot(string account) + { + return new RestartSettingsSnapshot( + new Settings.MainConfigHelper.MainConfig.AccountInfoConfig(account, "-"), + "localhost", + 25565); + } } diff --git a/MinecraftClient/Commands/Connect.cs b/MinecraftClient/Commands/Connect.cs index 61679e00..174fca22 100644 --- a/MinecraftClient/Commands/Connect.cs +++ b/MinecraftClient/Commands/Connect.cs @@ -47,8 +47,9 @@ namespace MinecraftClient.Commands if (Settings.Config.Main.SetServerIP(new Settings.MainConfigHelper.MainConfig.ServerInfoConfig(server), true)) { - Program.Restart(keepAccountAndServerSettings: true); - return r.SetAndReturn(Status.Done); + return r.SetAndReturn(Program.TryRestart(keepAccountAndServerSettings: true) + ? Status.Done + : Status.Fail); } else { @@ -64,8 +65,9 @@ namespace MinecraftClient.Commands if (Settings.Config.Main.SetServerIP(new Settings.MainConfigHelper.MainConfig.ServerInfoConfig(args[0]), true)) { - Program.Restart(keepAccountAndServerSettings: true); - return string.Empty; + return Program.TryRestart(keepAccountAndServerSettings: true) + ? string.Empty + : Translations.general_fail; } else { diff --git a/MinecraftClient/Commands/Reco.cs b/MinecraftClient/Commands/Reco.cs index 482b4e0b..006217ad 100644 --- a/MinecraftClient/Commands/Reco.cs +++ b/MinecraftClient/Commands/Reco.cs @@ -47,8 +47,9 @@ namespace MinecraftClient.Commands if (!Settings.Config.Main.Advanced.SetAccount(account)) return r.SetAndReturn(CmdResult.Status.Fail, string.Format(Translations.cmd_connect_unknown, account)); } - Program.Restart(keepAccountAndServerSettings: true); - return r.SetAndReturn(CmdResult.Status.Done); + return r.SetAndReturn(Program.TryRestart(keepAccountAndServerSettings: true) + ? CmdResult.Status.Done + : CmdResult.Status.Fail); } internal static string DoReconnect(string command) @@ -62,8 +63,9 @@ namespace MinecraftClient.Commands return string.Format(Translations.cmd_connect_unknown, account); } } - Program.Restart(keepAccountAndServerSettings: true); - return String.Empty; + return Program.TryRestart(keepAccountAndServerSettings: true) + ? String.Empty + : Translations.general_fail; } } } diff --git a/MinecraftClient/Program.cs b/MinecraftClient/Program.cs index 56d1d641..7da5bcfc 100644 --- a/MinecraftClient/Program.cs +++ b/MinecraftClient/Program.cs @@ -923,10 +923,15 @@ namespace MinecraftClient if (delay < TimeSpan.Zero) delay = TimeSpan.Zero; + RestartSettingsSnapshot? settingsSnapshot = keepAccountAndServerSettings + ? new RestartSettingsSnapshot(InternalConfig.Account, InternalConfig.ServerIP, InternalConfig.ServerPort) + : null; + return restartCoordinator.TrySchedule(new RestartRequest( Volatile.Read(ref connectionAttempt), delay, - keepAccountAndServerSettings)); + keepAccountAndServerSettings, + settingsSnapshot)); } private static async Task ExecuteRestartAsync(RestartRequest request, CancellationToken cancellationToken) @@ -951,6 +956,12 @@ namespace MinecraftClient cancellationToken.ThrowIfCancellationRequested(); ConsoleIO.WriteLine(Translations.mcc_restart); ReloadSettings(request.KeepAccountAndServerSettings); + if (request.SettingsSnapshot is RestartSettingsSnapshot settingsSnapshot) + { + InternalConfig.Account = settingsSnapshot.Account; + InternalConfig.ServerIP = settingsSnapshot.ServerIP; + InternalConfig.ServerPort = settingsSnapshot.ServerPort; + } InitializeClient(); } diff --git a/MinecraftClient/RestartCoordinator.cs b/MinecraftClient/RestartCoordinator.cs index 3193d933..193d454b 100644 --- a/MinecraftClient/RestartCoordinator.cs +++ b/MinecraftClient/RestartCoordinator.cs @@ -9,7 +9,14 @@ namespace MinecraftClient internal readonly record struct RestartRequest( long ConnectionAttempt, TimeSpan Delay, - bool KeepAccountAndServerSettings); + bool KeepAccountAndServerSettings, + RestartSettingsSnapshot? SettingsSnapshot = null, + long RequestId = 0); + + internal readonly record struct RestartSettingsSnapshot( + Settings.MainConfigHelper.MainConfig.AccountInfoConfig Account, + string ServerIP, + ushort ServerPort); internal sealed class RestartCoordinator : IDisposable { @@ -19,8 +26,9 @@ namespace MinecraftClient private readonly Func restart; private readonly Action reportFailure; private readonly Task worker; - private readonly HashSet pendingAttempts = []; + private readonly Dictionary pendingAttempts = []; private long highestScheduledAttempt = -1; + private long nextRequestId; private bool stopped; internal RestartCoordinator( @@ -44,22 +52,28 @@ namespace MinecraftClient internal bool HasScheduledRestart(long connectionAttempt) { lock (stateLock) - return !stopped && pendingAttempts.Contains(connectionAttempt); + return !stopped && pendingAttempts.ContainsKey(connectionAttempt); } internal bool TrySchedule(RestartRequest request) { lock (stateLock) { - if (stopped || request.ConnectionAttempt <= highestScheduledAttempt) + bool hasPendingRequest = pendingAttempts.TryGetValue(request.ConnectionAttempt, out long previousRequestId); + if (stopped || request.ConnectionAttempt < highestScheduledAttempt + || (request.ConnectionAttempt == highestScheduledAttempt && !hasPendingRequest)) return false; - highestScheduledAttempt = request.ConnectionAttempt; - pendingAttempts.Add(request.ConnectionAttempt); + highestScheduledAttempt = Math.Max(highestScheduledAttempt, request.ConnectionAttempt); + request = request with { RequestId = ++nextRequestId }; + pendingAttempts[request.ConnectionAttempt] = request.RequestId; if (requests.Writer.TryWrite(request)) return true; - pendingAttempts.Remove(request.ConnectionAttempt); + if (hasPendingRequest) + pendingAttempts[request.ConnectionAttempt] = previousRequestId; + else + pendingAttempts.Remove(request.ConnectionAttempt); return false; } } @@ -84,6 +98,13 @@ namespace MinecraftClient { await foreach (RestartRequest request in requests.Reader.ReadAllAsync(shutdown.Token).ConfigureAwait(false)) { + lock (stateLock) + { + if (!pendingAttempts.TryGetValue(request.ConnectionAttempt, out long requestId) + || requestId != request.RequestId) + continue; + } + try { await restart(request, shutdown.Token).ConfigureAwait(false); @@ -99,7 +120,11 @@ namespace MinecraftClient finally { lock (stateLock) - pendingAttempts.Remove(request.ConnectionAttempt); + { + if (pendingAttempts.TryGetValue(request.ConnectionAttempt, out long requestId) + && requestId == request.RequestId) + pendingAttempts.Remove(request.ConnectionAttempt); + } } } }