Fix exception while shutting down instance procedures

Agent/Phantom.Agent.Minecraft/Instance/InstanceProcess.cs

@@ -5,7 +5,6 @@ namespace Phantom.Agent.Minecraft.Instance;
 
 public sealed class InstanceProcess : IDisposable {
 	public InstanceProperties InstanceProperties { get; }
-	public CancellableSemaphore BackupSemaphore { get; } = new (1, 1);
 	
 	private readonly RingBuffer<string> outputBuffer = new (100);
 	private event EventHandler<string>? OutputEvent;
@@ -61,7 +60,6 @@ public sealed class InstanceProcess : IDisposable {
 
 	public void Dispose() {
 		process.Dispose();
-		BackupSemaphore.Dispose();
 		OutputEvent = null;
 		Ended = null;
 	}

Agent/Phantom.Agent.Services/Backups/BackupManager.cs

@@ -16,27 +16,13 @@ sealed class BackupManager : IDisposable {
 		this.compressionSemaphore = new SemaphoreSlim(maxConcurrentCompressionTasks, maxConcurrentCompressionTasks);
 	}
 
+	public Task<BackupCreationResult> CreateBackup(string loggerName, InstanceProcess process, CancellationToken cancellationToken) {
+		return new BackupCreator(this, loggerName, process, cancellationToken).CreateBackup();
+	}
+
 	public void Dispose() {
 		compressionSemaphore.Dispose();
 	}
-	
-	public async Task<BackupCreationResult> CreateBackup(string loggerName, InstanceProcess process, CancellationToken cancellationToken) {
-		try {
-			if (!await process.BackupSemaphore.Wait(TimeSpan.FromSeconds(1), cancellationToken)) {
-				return new BackupCreationResult(BackupCreationResultKind.BackupAlreadyRunning);
-			}
-		} catch (ObjectDisposedException) {
-			return new BackupCreationResult(BackupCreationResultKind.InstanceNotRunning);
-		} catch (OperationCanceledException) {
-			return new BackupCreationResult(BackupCreationResultKind.InstanceNotRunning);
-		}
-
-		try {
-			return await new BackupCreator(this, loggerName, process, cancellationToken).CreateBackup();
-		} finally {
-			process.BackupSemaphore.Release();
-		}
-	}
 
 	private sealed class BackupCreator {
 		private readonly BackupManager manager;
@@ -89,7 +75,7 @@ sealed class BackupManager : IDisposable {
 				try {
 					await dispatcher.EnableAutomaticSaving();
 				} catch (OperationCanceledException) {
-					// ignore
+					// Ignore.
 				} catch (Exception e) {
 					resultBuilder.Warnings |= BackupCreationWarnings.CouldNotRestoreAutomaticSaving;
 					logger.Error(e, "Caught exception while enabling automatic saving after creating an instance backup.");

Agent/Phantom.Agent.Services/Backups/BackupScheduler.cs

@@ -1,5 +1,7 @@
 using Phantom.Agent.Minecraft.Instance;
 using Phantom.Agent.Minecraft.Server;
+using Phantom.Agent.Services.Instances;
+using Phantom.Agent.Services.Instances.Procedures;
 using Phantom.Common.Data.Backups;
 using Phantom.Common.Logging;
 using Phantom.Utils.Runtime;
@@ -12,21 +14,22 @@ sealed class BackupScheduler : CancellableBackgroundTask {
 	private static readonly TimeSpan BackupInterval = TimeSpan.FromMinutes(30);
 	private static readonly TimeSpan BackupFailureRetryDelay = TimeSpan.FromMinutes(5);
 
-	private readonly string loggerName;
 	private readonly BackupManager backupManager;
 	private readonly InstanceProcess process;
+	private readonly IInstanceContext context;
+	private readonly SemaphoreSlim backupSemaphore = new (1, 1);
 	private readonly int serverPort;
 	private readonly ServerStatusProtocol serverStatusProtocol;
 	private readonly ManualResetEventSlim serverOutputWhileWaitingForOnlinePlayers = new ();
 	
 	public event EventHandler<BackupCreationResult>? BackupCompleted; 
 
-	public BackupScheduler(TaskManager taskManager, BackupManager backupManager, InstanceProcess process, int serverPort, string loggerName) : base(PhantomLogger.Create<BackupScheduler>(loggerName), taskManager, "Backup scheduler for " + loggerName) {
-		this.loggerName = loggerName;
+	public BackupScheduler(TaskManager taskManager, BackupManager backupManager, InstanceProcess process, IInstanceContext context, int serverPort) : base(PhantomLogger.Create<BackupScheduler>(context.ShortName), taskManager, "Backup scheduler for " + context.ShortName) {
 		this.backupManager = backupManager;
 		this.process = process;
+		this.context = context;
 		this.serverPort = serverPort;
-		this.serverStatusProtocol = new ServerStatusProtocol(loggerName);
+		this.serverStatusProtocol = new ServerStatusProtocol(context.ShortName);
 		Start();
 	}
 
@@ -51,7 +54,17 @@ sealed class BackupScheduler : CancellableBackgroundTask {
 	}
 
 	private async Task<BackupCreationResult> CreateBackup() {
-		return await backupManager.CreateBackup(loggerName, process, CancellationToken.None);
+		if (!await backupSemaphore.WaitAsync(TimeSpan.FromSeconds(1))) {
+			return new BackupCreationResult(BackupCreationResultKind.BackupAlreadyRunning);
+		}
+		
+		try {
+			var procedure = new BackupInstanceProcedure(backupManager);
+			context.EnqueueProcedure(procedure);
+			return await procedure.Result;
+		} finally {
+			backupSemaphore.Release();
+		}
 	}
 
 	private async Task WaitForOnlinePlayers() {
@@ -96,6 +109,7 @@ sealed class BackupScheduler : CancellableBackgroundTask {
 	}
 
 	protected override void Dispose() {
+		backupSemaphore.Dispose();
 		serverOutputWhileWaitingForOnlinePlayers.Dispose();
 	}
 }

Agent/Phantom.Agent.Services/Instances/Instance.cs

@@ -63,8 +63,10 @@ sealed class Instance : IAsyncDisposable {
 
 	private void ReportAndSetStatus(IInstanceStatus status) {
 		TryUpdateStatus("Report status of instance " + shortName + " as " + status.GetType().Name, async () => {
-			currentStatus = status;
-			await ServerMessaging.Send(new ReportInstanceStatusMessage(Configuration.InstanceGuid, status));
+			if (status != currentStatus) {
+				currentStatus = status;
+				await ServerMessaging.Send(new ReportInstanceStatusMessage(Configuration.InstanceGuid, status));
+			}
 		});
 	}
 
@@ -121,6 +123,7 @@ sealed class Instance : IAsyncDisposable {
 			configurationSemaphore.Release();
 		}
 		
+		ReportAndSetStatus(InstanceStatus.Launching);
 		await procedureManager.Enqueue(procedure);
 		return LaunchInstanceResult.LaunchInitiated;
 	}
@@ -134,6 +137,7 @@ sealed class Instance : IAsyncDisposable {
 			return StopInstanceResult.InstanceAlreadyStopping;
 		}
 		
+		ReportAndSetStatus(InstanceStatus.Stopping);
 		await procedureManager.Enqueue(new StopInstanceProcedure(stopStrategy));
 		return StopInstanceResult.StopInitiated;
 	}

Agent/Phantom.Agent.Services/Instances/InstanceProcedureManager.cs

@@ -32,10 +32,6 @@ sealed class InstanceProcedureManager : IAsyncDisposable {
 		return (await currentProcedure.Get(cancellationToken))?.Procedure;
 	}
 
-	public async Task CancelCurrentProcedure() {
-		(await currentProcedure.Get(shutdownCancellationTokenSource.Token))?.CancellationTokenSource.Cancel();
-	}
-
 	private async Task Run() {
 		try {
 			var shutdownCancellationToken = shutdownCancellationTokenSource.Token;
@@ -77,7 +73,7 @@ sealed class InstanceProcedureManager : IAsyncDisposable {
 	public async ValueTask DisposeAsync() {
 		shutdownCancellationTokenSource.Cancel();
 
-		await CancelCurrentProcedure();
+		(await currentProcedure.Get(CancellationToken.None))?.CancellationTokenSource.Cancel();
 		await procedureQueueFinished.WaitHandle.WaitOneAsync();
 
 		currentProcedure.Dispose();

Agent/Phantom.Agent.Services/Instances/Procedures/BackupInstanceProcedure.cs

@@ -0,0 +1,29 @@
+using Phantom.Agent.Services.Backups;
+using Phantom.Agent.Services.Instances.States;
+using Phantom.Common.Data.Backups;
+
+namespace Phantom.Agent.Services.Instances.Procedures;
+
+sealed record BackupInstanceProcedure(BackupManager BackupManager) : IInstanceProcedure {
+	private readonly TaskCompletionSource<BackupCreationResult> resultCompletionSource = new ();
+
+	public Task<BackupCreationResult> Result => resultCompletionSource.Task;
+
+	public async Task<IInstanceState?> Run(IInstanceContext context, CancellationToken cancellationToken) {
+		if (context.CurrentState is not InstanceRunningState runningState || runningState.Process.HasEnded) {
+			resultCompletionSource.SetResult(new BackupCreationResult(BackupCreationResultKind.InstanceNotRunning));
+			return null;
+		}
+
+		try {
+			var result = await BackupManager.CreateBackup(context.ShortName, runningState.Process, cancellationToken);
+			resultCompletionSource.SetResult(result);
+		} catch (OperationCanceledException) {
+			resultCompletionSource.SetCanceled(cancellationToken);
+		} catch (Exception e) {
+			resultCompletionSource.SetException(e);
+		}
+
+		return null;
+	}
+}

Agent/Phantom.Agent.Services/Instances/Procedures/StopInstanceProcedure.cs

@@ -33,6 +33,7 @@ sealed record StopInstanceProcedure(MinecraftStopStrategy StopStrategy) : IInsta
 		try {
 			// Too late to cancel the stop procedure now.
 			if (!process.HasEnded) {
+				context.Logger.Information("Session stopping now.");
 				await DoStop(context, process);
 			}
 		} finally {
@@ -66,14 +67,6 @@ sealed record StopInstanceProcedure(MinecraftStopStrategy StopStrategy) : IInsta
 	}
 
 	private async Task DoStop(IInstanceContext context, InstanceProcess process) {
-		context.Logger.Information("Session stopping now.");
-
-		// Do not release the semaphore after this point.
-		if (!await process.BackupSemaphore.CancelAndWait(TimeSpan.FromSeconds(1))) {
-			context.Logger.Information("Waiting for backup to finish...");
-			await process.BackupSemaphore.CancelAndWait(Timeout.InfiniteTimeSpan);
-		}
-
 		context.Logger.Information("Sending stop command...");
 		await TrySendStopCommand(context, process);
 

Agent/Phantom.Agent.Services/Instances/States/InstanceRunningState.cs

@@ -29,7 +29,7 @@ sealed class InstanceRunningState : IInstanceState, IDisposable {
 
 		this.logSender = new InstanceLogSender(context.Services.TaskManager, configuration.InstanceGuid, context.ShortName);
 
-		this.backupScheduler = new BackupScheduler(context.Services.TaskManager, context.Services.BackupManager, process, configuration.ServerPort, context.ShortName);
+		this.backupScheduler = new BackupScheduler(context.Services.TaskManager, context.Services.BackupManager, process, context, configuration.ServerPort);
 		this.backupScheduler.BackupCompleted += OnScheduledBackupCompleted;
 	}
 

Utils/Phantom.Utils.Runtime/CancellableSemaphore.cs

@@ -1,28 +0,0 @@
-namespace Phantom.Utils.Runtime; 
-
-public sealed class CancellableSemaphore : IDisposable {
-	private readonly SemaphoreSlim semaphore;
-	private readonly CancellationTokenSource cancellationTokenSource = new ();
-	
-	public CancellableSemaphore(int initialCount, int maxCount) {
-		this.semaphore = new SemaphoreSlim(initialCount, maxCount);
-	}
-
-	public async Task<bool> Wait(TimeSpan timeout, CancellationToken cancellationToken) {
-		return await semaphore.WaitAsync(timeout, cancellationTokenSource.Token).WaitAsync(cancellationToken);
-	}
-
-	public async Task<bool> CancelAndWait(TimeSpan timeout) {
-		cancellationTokenSource.Cancel();
-		return await semaphore.WaitAsync(timeout);
-	}
-
-	public void Release() {
-		semaphore.Release();
-	}
-	
-	public void Dispose() {
-		semaphore.Dispose();
-		cancellationTokenSource.Dispose();
-	}
-}