Fix: make AutoRelog retries single-owner (#3186)

Login rejections could schedule and execute multiple restarts for one failure while leaving no usable console route during reconnect delays.

Changes:
- Bind failure and restart work to immutable connection attempts
- Coalesce automatic retries while allowing explicit settings replacement until commit
- Preserve held bots and offline command routing across failed logins
- Add deterministic retry, ownership, and routing regression tests

Fixes #3186
This commit is contained in:
Anon 2026-07-27 16:12:10 +02:00
parent 92212d2b95
commit 456a548cbc
11 changed files with 645 additions and 114 deletions

View file

@ -62,6 +62,37 @@ public sealed class AutoRelogRetryPolicyTests
Assert.Equal(0, retriesLeft);
}
[Fact]
public void CoalescedDuplicateRollsBackOnlyItsOwnReservation()
{
var policy = new AutoRelogRetryPolicy(new ManualTimeProvider());
Assert.True(policy.TryReserveAttempt(2, out int firstRetriesLeft));
Assert.True(policy.TryReserveAttempt(2, out int duplicateRetriesLeft));
policy.RollBackReservedAttempt();
Assert.Equal(1, firstRetriesLeft);
Assert.Equal(0, duplicateRetriesLeft);
Assert.Equal(1, policy.Attempts);
Assert.True(policy.TryReserveAttempt(2, out int secondFailureRetriesLeft));
Assert.Equal(0, secondFailureRetriesLeft);
Assert.False(policy.TryReserveAttempt(2, out _));
}
[Fact]
public void UnlimitedDuplicateRollbackKeepsUnlimitedBudget()
{
var policy = new AutoRelogRetryPolicy(new ManualTimeProvider());
Assert.True(policy.TryReserveAttempt(-1, out _));
Assert.True(policy.TryReserveAttempt(-1, out _));
policy.RollBackReservedAttempt();
Assert.Equal(1, policy.Attempts);
Assert.True(policy.TryReserveAttempt(-1, out int retriesLeft));
Assert.Equal(-1, retriesLeft);
}
[Fact]
public void StableConnectionResetsRetryBudget()
{

View file

@ -0,0 +1,110 @@
using MinecraftClient.Scripting;
namespace MinecraftClient.Tests;
public sealed class McClientConnectionFailureTests
{
[Fact]
public void LoginRejectedClaimPreventsSyntheticConnectionLostFallback()
{
var lifecycle = new ConnectionAttemptLifecycle();
Assert.True(lifecycle.TryBeginDisconnect());
Assert.True(lifecycle.IsFailureClaimed);
Assert.False(lifecycle.TryBeginDisconnect());
lifecycle.CompleteDisconnect();
Assert.True(lifecycle.IsFailureClaimed);
Assert.False(lifecycle.TryBeginDisconnect());
}
[Fact]
public void UnclaimedGenericFailureCanBeClaimedExactlyOnce()
{
var lifecycle = new ConnectionAttemptLifecycle();
Assert.False(lifecycle.IsFailureClaimed);
Assert.True(lifecycle.TryBeginDisconnect());
Assert.False(lifecycle.TryBeginDisconnect());
}
[Fact]
public void HeldBotsAreRestoredBeforeFailureAndReceiveOriginalMessageOnce()
{
const string rejectionMessage = "You are not white-listed on this server!";
var bot = new RecordingBot();
List<ChatBot> heldBots = [bot];
var loadedBots = new List<ChatBot>();
ConnectionAttemptLifecycle.RestoreHeldBots(heldBots, loadedBots.Add);
foreach (ChatBot loadedBot in loadedBots)
loadedBot.OnDisconnect(ChatBot.DisconnectReason.LoginRejected, rejectionMessage);
Assert.Empty(heldBots);
Assert.Single(loadedBots);
Assert.Equal(1, bot.DisconnectCount);
Assert.Equal(ChatBot.DisconnectReason.LoginRejected, bot.LastReason);
Assert.Equal(rejectionMessage, bot.LastMessage);
}
[Fact]
public void OfflineRouteStaysOwnedAcrossReplacementAndSuccessfulHandoff()
{
var route = new AttemptOwnedRoute();
int activations = 0;
int deactivations = 0;
Assert.True(route.TryActivate(7, () => activations++));
Assert.False(route.TryActivate(7, () => activations++));
Assert.True(route.TryTransfer(7, 8));
Assert.False(route.TryDeactivate(7, () => deactivations++));
Assert.Equal(8, route.OwnerAttempt);
Assert.True(route.TryDeactivate(8, () => deactivations++));
Assert.Equal(1, activations);
Assert.Equal(1, deactivations);
Assert.Equal(-1, route.OwnerAttempt);
}
[Fact]
public void InitialConnectionAttemptCanOwnOfflineRoute()
{
var route = new AttemptOwnedRoute();
int activations = 0;
Assert.True(route.TryActivate(0, () => activations++));
Assert.Equal(1, activations);
Assert.Equal(0, route.OwnerAttempt);
}
[Fact]
public void StaleCleanupCannotClearNewerOfflineRoute()
{
var route = new AttemptOwnedRoute();
int deactivations = 0;
Assert.True(route.TryActivate(10, () => { }));
Assert.True(route.TryActivate(11, () => { }));
Assert.False(route.TryDeactivate(10, () => deactivations++));
Assert.Equal(11, route.OwnerAttempt);
Assert.Equal(0, deactivations);
}
private sealed class RecordingBot : ChatBot
{
internal int DisconnectCount { get; private set; }
internal DisconnectReason? LastReason { get; private set; }
internal string? LastMessage { get; private set; }
public override bool OnDisconnect(DisconnectReason reason, string message)
{
DisconnectCount++;
LastReason = reason;
LastMessage = message;
return false;
}
}
}

View file

@ -3,55 +3,163 @@ namespace MinecraftClient.Tests;
public sealed class RestartCoordinatorTests
{
[Fact]
public async Task ReplacesQueuedSameAttemptAndQueuesNewerAttempt()
public async Task AutomaticSameAttemptIsCoalescedWhileQueued()
{
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<string>();
var blockerStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
var releaseBlocker = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
var queuedCompleted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
int queuedExecutions = 0;
using var coordinator = new RestartCoordinator(
RestartCoordinator coordinator = null!;
coordinator = new RestartCoordinator(
async (request, cancellationToken) =>
{
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();
blockerStarted.SetResult();
await releaseBlocker.Task.WaitAsync(cancellationToken);
return;
}
Interlocked.Increment(ref queuedExecutions);
Assert.True(coordinator.TryBeginCommit(request, out _));
queuedCompleted.SetResult();
},
exception => throw new Xunit.Sdk.XunitException(exception.ToString()));
using var cleanup = coordinator;
Assert.True(coordinator.TrySchedule(new RestartRequest(9, TimeSpan.Zero, true)));
await firstStarted.Task.WaitAsync(TimeSpan.FromSeconds(5));
await blockerStarted.Task.WaitAsync(TimeSpan.FromSeconds(5));
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));
Assert.True(coordinator.TrySchedule(new RestartRequest(10, TimeSpan.Zero, true)));
Assert.False(coordinator.TrySchedule(new RestartRequest(10, TimeSpan.Zero, true)));
Assert.True(coordinator.HasScheduledRestart(10));
releaseFirst.SetResult();
await replacementCompleted.Task.WaitAsync(TimeSpan.FromSeconds(5));
await secondCompleted.Task.WaitAsync(TimeSpan.FromSeconds(5));
releaseBlocker.SetResult();
await queuedCompleted.Task.WaitAsync(TimeSpan.FromSeconds(5));
Assert.Equal(["replacement"], executedAccounts);
Assert.Equal(1, queuedExecutions);
}
[Fact]
public async Task AutomaticSameAttemptIsCoalescedDuringCallback()
{
var callbackStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
var releaseCallback = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
var callbackCompleted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
int executions = 0;
RestartCoordinator coordinator = null!;
coordinator = new RestartCoordinator(
async (request, cancellationToken) =>
{
Interlocked.Increment(ref executions);
callbackStarted.SetResult();
await releaseCallback.Task.WaitAsync(cancellationToken);
Assert.True(coordinator.TryBeginCommit(request, out _));
callbackCompleted.SetResult();
},
exception => throw new Xunit.Sdk.XunitException(exception.ToString()));
using var cleanup = coordinator;
Assert.True(coordinator.TrySchedule(new RestartRequest(42, TimeSpan.Zero, true)));
await callbackStarted.Task.WaitAsync(TimeSpan.FromSeconds(5));
Assert.False(coordinator.TrySchedule(new RestartRequest(42, TimeSpan.Zero, true)));
releaseCallback.SetResult();
await callbackCompleted.Task.WaitAsync(TimeSpan.FromSeconds(5));
Assert.Equal(1, executions);
}
[Fact]
public async Task ExplicitReplacementDuringDelayUsesLatestSnapshotWithoutAnotherExecution()
{
var callbackStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
var allowCommit = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
var callbackCompleted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
RestartRequest committedRequest = default;
int executions = 0;
RestartCoordinator coordinator = null!;
coordinator = new RestartCoordinator(
async (request, cancellationToken) =>
{
Interlocked.Increment(ref executions);
callbackStarted.SetResult();
await allowCommit.Task.WaitAsync(cancellationToken);
Assert.True(coordinator.TryBeginCommit(request, out committedRequest));
callbackCompleted.SetResult();
},
exception => throw new Xunit.Sdk.XunitException(exception.ToString()));
using var cleanup = coordinator;
Assert.True(coordinator.TrySchedule(new RestartRequest(
50,
TimeSpan.FromSeconds(10),
true,
CreateSettingsSnapshot("first"))));
await callbackStarted.Task.WaitAsync(TimeSpan.FromSeconds(5));
Assert.True(coordinator.TrySchedule(new RestartRequest(
50,
TimeSpan.Zero,
true,
CreateSettingsSnapshot("replacement"),
ReplaceUntilCommit: true)));
allowCommit.SetResult();
await callbackCompleted.Task.WaitAsync(TimeSpan.FromSeconds(5));
Assert.Equal(1, executions);
Assert.Equal("replacement", committedRequest.SettingsSnapshot?.Account.Login);
}
[Fact]
public async Task RejectsSameAttemptReplacementAfterCommit()
{
var commitStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
var releaseCommit = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
RestartRequest committedRequest = default;
RestartCoordinator coordinator = null!;
coordinator = new RestartCoordinator(
async (request, cancellationToken) =>
{
Assert.True(coordinator.TryBeginCommit(request, out committedRequest));
commitStarted.SetResult();
await releaseCommit.Task.WaitAsync(cancellationToken);
},
exception => throw new Xunit.Sdk.XunitException(exception.ToString()));
using var cleanup = coordinator;
Assert.True(coordinator.TrySchedule(new RestartRequest(
60,
TimeSpan.Zero,
true,
CreateSettingsSnapshot("committed"))));
await commitStarted.Task.WaitAsync(TimeSpan.FromSeconds(5));
Assert.False(coordinator.TrySchedule(new RestartRequest(
60,
TimeSpan.Zero,
true,
CreateSettingsSnapshot("rejected"),
ReplaceUntilCommit: true)));
Assert.Equal("committed", committedRequest.SettingsSnapshot?.Account.Login);
releaseCommit.SetResult();
}
[Fact]
public void RejectsStaleAttempt()
{
using var coordinator = new RestartCoordinator(
RestartCoordinator coordinator = null!;
coordinator = new RestartCoordinator(
(_, _) => Task.CompletedTask,
exception => throw new Xunit.Sdk.XunitException(exception.ToString()));
using var cleanup = coordinator;
Assert.True(coordinator.TrySchedule(new RestartRequest(20, TimeSpan.Zero, true)));
Assert.False(coordinator.TrySchedule(new RestartRequest(19, TimeSpan.Zero, true)));
@ -61,13 +169,16 @@ public sealed class RestartCoordinatorTests
public async Task RejectsCompletedAttempt()
{
var completed = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
using var coordinator = new RestartCoordinator(
(_, _) =>
RestartCoordinator coordinator = null!;
coordinator = new RestartCoordinator(
(request, cancellationToken) =>
{
Assert.True(coordinator.TryBeginCommit(request, out _));
completed.SetResult();
return Task.CompletedTask;
},
exception => throw new Xunit.Sdk.XunitException(exception.ToString()));
using var cleanup = coordinator;
Assert.True(coordinator.TrySchedule(new RestartRequest(20, TimeSpan.Zero, true)));
await completed.Task.WaitAsync(TimeSpan.FromSeconds(5));
@ -77,15 +188,23 @@ public sealed class RestartCoordinatorTests
}
[Fact]
public void TerminalStopRejectsFurtherRestarts()
public async Task TerminalStopCancelsInFlightWorkAndRejectsFurtherRestarts()
{
var callbackStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
using var coordinator = new RestartCoordinator(
(_, _) => Task.CompletedTask,
async (_, cancellationToken) =>
{
callbackStarted.SetResult();
await Task.Delay(Timeout.InfiniteTimeSpan, cancellationToken);
},
exception => throw new Xunit.Sdk.XunitException(exception.ToString()));
Assert.True(coordinator.TrySchedule(new RestartRequest(1, TimeSpan.Zero, true)));
await callbackStarted.Task.WaitAsync(TimeSpan.FromSeconds(5));
coordinator.Stop();
Assert.False(coordinator.TrySchedule(new RestartRequest(1, TimeSpan.Zero, true)));
Assert.False(coordinator.TrySchedule(new RestartRequest(2, TimeSpan.Zero, true)));
Assert.False(coordinator.HasScheduledRestart(1));
}