Skip to content

chore: Extract server recovery tracking from controller #1552

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hatayama merged 1 commit into from
Jul 6, 2026
Merged

chore: Extract server recovery tracking from controller #1552

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 17 additions & 10 deletions Assets/Tests/Editor/UnityCliLoopServerControllerStartupLockTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public void ScheduleStartupRecovery_WhenCalled_ExposesRecoveryTaskBeforeDeferred
// Tests that deferred startup recovery exposes its pending task before execution.
System.Action scheduledAction = null;
bool recoveryExecuted = false;
UnityCliLoopServerControllerService service = CreateControllerService();
UnityCliLoopServerRecoveryTrackingService service = CreateRecoveryTrackingService();

Task recoveryTask = service.ScheduleStartupRecovery(
action => scheduledAction = action,
Expand All @@ -66,7 +66,7 @@ public void ScheduleStartupRecovery_WhenRecoveryThrowsSynchronously_FaultsTaskAn
{
// Tests that synchronous startup recovery failures fault and clear the tracked task.
System.Action scheduledAction = null;
UnityCliLoopServerControllerService service = CreateControllerService();
UnityCliLoopServerRecoveryTrackingService service = CreateRecoveryTrackingService();

Task recoveryTask = service.ScheduleStartupRecovery(
action => scheduledAction = action,
Expand All @@ -85,7 +85,7 @@ public async Task ScheduleStartupRecovery_WhenRecoveryIsAsync_KeepsTaskIncomplet
// Tests that asynchronous startup recovery remains pending until its restore task completes.
System.Action scheduledAction = null;
TaskCompletionSource<bool> recoveryCompletionSource = new();
UnityCliLoopServerControllerService service = CreateControllerService();
UnityCliLoopServerRecoveryTrackingService service = CreateRecoveryTrackingService();

Task recoveryTask = service.ScheduleStartupRecovery(
action => scheduledAction = action,
Expand Down Expand Up @@ -225,8 +225,7 @@ public async Task ExecuteTrackedRecoveryAsync_WhenRecoveryFailsOnce_RetriesAfter
// is retried with backoff instead of leaving the server down until the next domain reload.
System.Collections.Generic.List<int> recordedWaits = new();
int recoveryAttempts = 0;
UnityCliLoopServerControllerService service = CreateControllerService(
new TestReadinessProbe(),
UnityCliLoopServerRecoveryTrackingService service = CreateRecoveryTrackingService(
(delayMilliseconds, ct) =>
{
recordedWaits.Add(delayMilliseconds);
Expand Down Expand Up @@ -259,8 +258,7 @@ public void ExecuteTrackedRecoveryAsync_WhenRecoveryKeepsFailing_GivesUpAfterAll
_sessionFlagsRepository.MarkServerStarted();
System.Collections.Generic.List<int> recordedWaits = new();
int recoveryAttempts = 0;
UnityCliLoopServerControllerService service = CreateControllerService(
new TestReadinessProbe(),
UnityCliLoopServerRecoveryTrackingService service = CreateRecoveryTrackingService(
(delayMilliseconds, ct) =>
{
recordedWaits.Add(delayMilliseconds);
Expand Down Expand Up @@ -291,8 +289,7 @@ public async Task ExecuteTrackedRecoveryAsync_WhenServerManuallyStoppedDuringBac
{
// Tests that an explicit Stop Server during the retry backoff wins over automatic recovery.
int recoveryAttempts = 0;
UnityCliLoopServerControllerService service = CreateControllerService(
new TestReadinessProbe(),
UnityCliLoopServerRecoveryTrackingService service = CreateRecoveryTrackingService(
(delayMilliseconds, ct) =>
{
_sessionFlagsRepository.MarkServerManuallyStopped();
Expand Down Expand Up @@ -424,6 +421,8 @@ private UnityCliLoopServerControllerService CreateControllerService(
waitBeforeReadinessRetryAsync,
readinessIdleTimeoutMilliseconds);
UnityCliLoopServerStartupProtectionService startupProtectionService = new();
UnityCliLoopServerRecoveryTrackingService recoveryTrackingService = CreateRecoveryTrackingService(
waitBeforeRecoveryRetryAsync);
return new UnityCliLoopServerControllerService(
effectiveServerInstanceFactory,
effectiveLifecycleRegistry,
Expand All @@ -434,7 +433,15 @@ private UnityCliLoopServerControllerService CreateControllerService(
domainReloadRecoveryUseCase,
readinessService,
startupProtectionService,
new TestDomainReloadLifecycle(),
recoveryTrackingService,
new TestDomainReloadLifecycle());
}

private UnityCliLoopServerRecoveryTrackingService CreateRecoveryTrackingService(
System.Func<int, CancellationToken, Task> waitBeforeRecoveryRetryAsync = null)
{
return new UnityCliLoopServerRecoveryTrackingService(
_sessionFlagsRepository,
waitBeforeRecoveryRetryAsync);
}

Expand Down
2 changes: 2 additions & 0 deletions Assets/Tests/Editor/UnityCliLoopServerStartupProtectionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ private UnityCliLoopServerControllerService CreateControllerService(
UnityCliLoopServerReadinessService readinessService = new(
lifecycleRegistry,
new TestReadinessProbe());
UnityCliLoopServerRecoveryTrackingService recoveryTrackingService = new(_sessionFlagsRepository);
return new UnityCliLoopServerControllerService(
serverInstanceFactory,
lifecycleRegistry,
Expand All @@ -137,6 +138,7 @@ private UnityCliLoopServerControllerService CreateControllerService(
domainReloadRecoveryUseCase,
readinessService,
startupProtectionService ?? new UnityCliLoopServerStartupProtectionService(),
recoveryTrackingService,
domainReloadLifecycle ?? new TestDomainReloadLifecycle());
}

Expand Down
2 changes: 2 additions & 0 deletions Packages/src/Editor/CompositionRoot/UnityCliLoopApplicationRegistration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ internal UnityCliLoopApplicationServices Register()
lifecycleRegistry,
firstPartyServerLifecycle);
UnityCliLoopServerStartupProtectionService startupProtectionService = new();
UnityCliLoopServerRecoveryTrackingService recoveryTrackingService = new(sessionFlagsRepository);
UnityCliLoopServerControllerService controllerService = new(
serverFactory,
lifecycleRegistry,
Expand All @@ -89,6 +90,7 @@ internal UnityCliLoopApplicationServices Register()
domainReloadRecoveryUseCase,
serverReadinessService,
startupProtectionService,
recoveryTrackingService,
firstPartyServerLifecycle);
UnityCliLoopServerApplicationService applicationService = new(controllerService);
UnityCliLoopServerApplicationFacade.RegisterService(applicationService);
Expand Down
162 changes: 9 additions & 153 deletions Packages/src/Editor/Infrastructure/Server/UnityCliLoopServerController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,10 @@ private enum ServerStopIntent
private readonly DomainReloadRecoveryUseCase _domainReloadRecoveryUseCase;
private readonly UnityCliLoopServerReadinessService _readinessService;
private readonly UnityCliLoopServerStartupProtectionService _startupProtectionService;
private readonly UnityCliLoopServerRecoveryTrackingService _recoveryTrackingService;
private readonly IUnityCliLoopServerDomainReloadLifecycle _domainReloadLifecycle;
private readonly Func<int, CancellationToken, Task> _waitBeforeRecoveryRetryAsync;
private IUnityCliLoopServerInstance _bridgeServer;
private readonly SemaphoreSlim _startupSemaphore = new SemaphoreSlim(1, 1);
private Task _currentRecoveryTask;

internal UnityCliLoopServerControllerService(
IUnityCliLoopServerInstanceFactory serverInstanceFactory,
Expand All @@ -50,8 +49,8 @@ internal UnityCliLoopServerControllerService(
DomainReloadRecoveryUseCase domainReloadRecoveryUseCase,
UnityCliLoopServerReadinessService readinessService,
UnityCliLoopServerStartupProtectionService startupProtectionService,
IUnityCliLoopServerDomainReloadLifecycle domainReloadLifecycle,
Func<int, CancellationToken, Task> waitBeforeRecoveryRetryAsync = null)
UnityCliLoopServerRecoveryTrackingService recoveryTrackingService,
IUnityCliLoopServerDomainReloadLifecycle domainReloadLifecycle)
{
System.Diagnostics.Debug.Assert(serverInstanceFactory != null, "serverInstanceFactory must not be null");
System.Diagnostics.Debug.Assert(serverLifecycleRegistry != null, "serverLifecycleRegistry must not be null");
Expand All @@ -62,6 +61,7 @@ internal UnityCliLoopServerControllerService(
System.Diagnostics.Debug.Assert(domainReloadRecoveryUseCase != null, "domainReloadRecoveryUseCase must not be null");
System.Diagnostics.Debug.Assert(readinessService != null, "readinessService must not be null");
System.Diagnostics.Debug.Assert(startupProtectionService != null, "startupProtectionService must not be null");
System.Diagnostics.Debug.Assert(recoveryTrackingService != null, "recoveryTrackingService must not be null");
System.Diagnostics.Debug.Assert(domainReloadLifecycle != null, "domainReloadLifecycle must not be null");

_serverInstanceFactory = serverInstanceFactory ?? throw new ArgumentNullException(nameof(serverInstanceFactory));
Expand All @@ -73,8 +73,8 @@ internal UnityCliLoopServerControllerService(
_domainReloadRecoveryUseCase = domainReloadRecoveryUseCase ?? throw new ArgumentNullException(nameof(domainReloadRecoveryUseCase));
_readinessService = readinessService ?? throw new ArgumentNullException(nameof(readinessService));
_startupProtectionService = startupProtectionService ?? throw new ArgumentNullException(nameof(startupProtectionService));
_recoveryTrackingService = recoveryTrackingService ?? throw new ArgumentNullException(nameof(recoveryTrackingService));
_domainReloadLifecycle = domainReloadLifecycle ?? throw new ArgumentNullException(nameof(domainReloadLifecycle));
_waitBeforeRecoveryRetryAsync = waitBeforeRecoveryRetryAsync ?? TimerDelay.Wait;
}

private bool IsBackgroundUnityProcess()
Expand Down Expand Up @@ -104,7 +104,7 @@ internal void RegisterRecoveredServer(IUnityCliLoopServerInstance server)
/// <summary>
/// Current recovery task. Can be awaited by other components to ensure recovery completes first.
/// </summary>
public Task RecoveryTask => _currentRecoveryTask;
public Task RecoveryTask => _recoveryTrackingService.RecoveryTask;

public void InitializeForEditorStartup()
{
Expand Down Expand Up @@ -132,75 +132,11 @@ public void InitializeForEditorStartup()

// Recovery binds the project IPC endpoint and may touch config files, so keep it off the
// synchronous Editor startup path while preserving automatic startup.
ScheduleStartupRecovery(
_recoveryTrackingService.ScheduleStartupRecovery(
action => EditorApplication.delayCall += () => action(),
RestoreServerStateIfNeeded);
}

internal Task ScheduleStartupRecovery(
Action<Action> scheduleDelayCall,
Func<Task> restoreServerState)
{
Debug.Assert(scheduleDelayCall != null, "scheduleDelayCall must not be null");
Debug.Assert(restoreServerState != null, "restoreServerState must not be null");

TaskCompletionSource<bool> scheduledRecoveryCompletionSource = new();
_currentRecoveryTask = scheduledRecoveryCompletionSource.Task;

scheduleDelayCall(() =>
{
Task restoreTask;
try
{
restoreTask = restoreServerState();
}
catch (Exception ex)
{
CompleteScheduledStartupRecovery(Task.FromException(ex), scheduledRecoveryCompletionSource);
return;
}

if (restoreTask.IsCompleted)
{
CompleteScheduledStartupRecovery(restoreTask, scheduledRecoveryCompletionSource);
return;
}

_ = restoreTask.ContinueWith(task =>
{
CompleteScheduledStartupRecovery(task, scheduledRecoveryCompletionSource);
}, CancellationToken.None, TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.FromCurrentSynchronizationContext());
});

return scheduledRecoveryCompletionSource.Task;
}

private void CompleteScheduledStartupRecovery(
Task restoreTask,
TaskCompletionSource<bool> scheduledRecoveryCompletionSource)
{
if (ReferenceEquals(_currentRecoveryTask, scheduledRecoveryCompletionSource.Task))
{
_currentRecoveryTask = null;
}

if (restoreTask.IsCanceled)
{
scheduledRecoveryCompletionSource.SetCanceled();
return;
}

if (restoreTask.IsFaulted)
{
VibeLogger.LogError("server_startup_restore_failed",
$"Failed to restore server: {restoreTask.Exception?.GetBaseException().Message}");
scheduledRecoveryCompletionSource.SetException(restoreTask.Exception.GetBaseException());
return;
}

scheduledRecoveryCompletionSource.SetResult(true);
}

public void StartServer()
{
// Why: an async void body lets exceptions (e.g. a readiness probe failure thrown by
Expand Down Expand Up @@ -335,7 +271,7 @@ internal void OnBeforeAssemblyReload()
/// </summary>
private void OnAfterAssemblyReload()
{
ScheduleTrackedRecovery(() => ExecuteAfterDomainReloadRecoveryAsync(CancellationToken.None));
_recoveryTrackingService.ScheduleTrackedRecovery(() => ExecuteAfterDomainReloadRecoveryAsync(CancellationToken.None));
}

private async Task ExecuteAfterDomainReloadRecoveryAsync(CancellationToken cancellationToken)
Expand Down Expand Up @@ -430,90 +366,10 @@ private void OnServerLoopUnexpectedlyExited()
// within the 5-second protection window after a successful start
_startupProtectionService.ClearStartupProtection();

ScheduleTrackedRecovery(() => StartRecoveryIfNeededAsync(false, CancellationToken.None));
_recoveryTrackingService.ScheduleTrackedRecovery(() => StartRecoveryIfNeededAsync(false, CancellationToken.None));
};
}

private Task ScheduleTrackedRecovery(Func<Task> recoveryAction)
{
Debug.Assert(recoveryAction != null, "recoveryAction must not be null");

Task recoveryTask = ExecuteTrackedRecoveryAsync(recoveryAction);
_currentRecoveryTask = recoveryTask;
_ = ClearTrackedRecoveryWhenCompleteAsync(recoveryTask);
return recoveryTask;
}

/// <summary>
/// Runs a recovery action, retrying with backoff so one transient failure
/// (e.g. readiness timeout during a heavy import) does not leave the server
/// down until the next domain reload.
/// </summary>
internal async Task ExecuteTrackedRecoveryAsync(Func<Task> recoveryAction)
{
Debug.Assert(recoveryAction != null, "recoveryAction must not be null");

int failedAttemptCount = 0;
while (true)
{
try
{
await recoveryAction();
return;
}
catch (Exception ex)
{
if (failedAttemptCount >= UnityCliLoopServerConfig.RECOVERY_RETRY_DELAYS_MS.Length)
{
string message = $"Unity CLI Loop server recovery failed before the bridge became ready. {ex.GetBaseException().Message}";
// Why: the thrown exception ends in an unobserved task and VibeLogger is
// compiled out without ULOOP_DEBUG, so without this console entry an
// unrecoverable server (uloop unreachable) would be completely silent.
Debug.LogError($"[{UnityCliLoopConstants.PROJECT_NAME}] {message}");
VibeLogger.LogError(
"server_recovery_failed",
message);
_sessionFlagsRepository.ClearServerSession();
throw new InvalidOperationException(message, ex);
}

int delayMilliseconds = UnityCliLoopServerConfig.RECOVERY_RETRY_DELAYS_MS[failedAttemptCount];
failedAttemptCount++;
VibeLogger.LogWarning(
"server_recovery_retry_scheduled",
$"Recovery attempt {failedAttemptCount} failed; retrying in {delayMilliseconds}ms. {ex.GetBaseException().Message}");
await _waitBeforeRecoveryRetryAsync(delayMilliseconds, CancellationToken.None);

// Why: an explicit Stop Server issued during the backoff must win over
// automatic recovery, otherwise the retry would silently restart the server.
if (_sessionFlagsRepository.GetIsServerManuallyStopped())
{
VibeLogger.LogInfo(
"server_recovery_retry_abandoned",
"Recovery retry abandoned because the server was manually stopped.");
return;
}
}
}
}

private async Task ClearTrackedRecoveryWhenCompleteAsync(Task recoveryTask)
{
Debug.Assert(recoveryTask != null, "recoveryTask must not be null");

try
{
await recoveryTask;
}
finally
{
if (ReferenceEquals(_currentRecoveryTask, recoveryTask))
{
_currentRecoveryTask = null;
}
}
}

/// <summary>
/// Centralized, coalesced recovery start.
/// Attempts recovery on the project IPC endpoint for up to 5 seconds.
Expand Down
Loading
Loading