Fixes #1548

src/Tgstation.Server.Host/Components/Session/ISessionController.cs

@@ -54,6 +54,11 @@ namespace Tgstation.Server.Host.Components.Session
 		/// </summary>
 		RebootState RebootState { get; }
 
+		/// <summary>
+		/// A <see cref="Task"/> that completes when the server calls /world/TgsNew().
+		/// </summary>
+		Task OnStartup { get; }
+
 		/// <summary>
 		/// A <see cref="Task"/> that completes when the server calls /world/TgsReboot().
 		/// </summary>
@@ -113,6 +118,7 @@ namespace Tgstation.Server.Host.Components.Session
 		/// Replace the <see cref="IDmbProvider"/> in use with a given <paramref name="newProvider"/>, disposing the old one.
 		/// </summary>
 		/// <param name="newProvider">The new <see cref="IDmbProvider"/>.</param>
-		void ReplaceDmbProvider(IDmbProvider newProvider);
+		/// <returns>An <see cref="IDisposable"/> to be disposed once certain that the original <see cref="IDmbProvider"/> is no longer in use.</returns>
+		IDisposable ReplaceDmbProvider(IDmbProvider newProvider);
 	}
 }

src/Tgstation.Server.Host/Components/Session/SessionController.cs

@@ -69,6 +69,9 @@ namespace Tgstation.Server.Host.Components.Session
 		/// <inheritdoc />
 		public Task<int> Lifetime { get; }
 
+		/// <inheritdoc />
+		public Task OnStartup => startupTcs.Task;
+
 		/// <inheritdoc />
 		public Task OnReboot => rebootTcs.Task;
 
@@ -143,20 +146,30 @@ namespace Tgstation.Server.Host.Components.Session
 		/// </summary>
 		TaskCompletionSource<bool> portAssignmentTcs;
 
+		/// <summary>
+		/// The <see cref="TaskCompletionSource"/> that completes when DD sends a valid startup bridge request.
+		/// </summary>
+		volatile TaskCompletionSource startupTcs;
+
+		/// <summary>
+		/// The <see cref="TaskCompletionSource"/> that completes when DD tells us about a reboot.
+		/// </summary>
+		volatile TaskCompletionSource rebootTcs;
+
+		/// <summary>
+		/// The <see cref="TaskCompletionSource"/> that completes when DD tells us it's primed.
+		/// </summary>
+		volatile TaskCompletionSource primeTcs;
+
 		/// <summary>
 		/// The port to assign DreamDaemon when it queries for it.
 		/// </summary>
 		ushort? nextPort;
 
 		/// <summary>
-		/// The <see cref="TaskCompletionSource"/> that completes when DD tells us about a reboot.
+		/// The <see cref="ApiValidationStatus"/> for the <see cref="SessionController"/>.
 		/// </summary>
-		TaskCompletionSource rebootTcs;
-
-		/// <summary>
-		/// The <see cref="TaskCompletionSource"/> that completes when DD tells us it's primed.
-		/// </summary>
-		TaskCompletionSource primeTcs;
+		ApiValidationStatus apiValidationStatus;
 
 		/// <summary>
 		/// If we know DreamDaemon currently has it's port closed.
@@ -168,11 +181,6 @@ namespace Tgstation.Server.Host.Components.Session
 		/// </summary>
 		bool disposed;
 
-		/// <summary>
-		/// The <see cref="ApiValidationStatus"/> for the <see cref="SessionController"/>.
-		/// </summary>
-		ApiValidationStatus apiValidationStatus;
-
 		/// <summary>
 		/// If <see cref="process"/> should be kept alive instead.
 		/// </summary>
@@ -234,6 +242,7 @@ namespace Tgstation.Server.Host.Components.Session
 			apiValidationStatus = ApiValidationStatus.NeverValidated;
 			released = false;
 
+			startupTcs = new TaskCompletionSource();
 			rebootTcs = new TaskCompletionSource();
 			primeTcs = new TaskCompletionSource();
 
@@ -500,11 +509,11 @@ namespace Tgstation.Server.Host.Components.Session
 		public void Resume() => process.Resume();
 
 		/// <inheritdoc />
-		public void ReplaceDmbProvider(IDmbProvider dmbProvider)
+		public IDisposable ReplaceDmbProvider(IDmbProvider dmbProvider)
 		{
 			var oldDmb = ReattachInformation.Dmb;
 			ReattachInformation.Dmb = dmbProvider ?? throw new ArgumentNullException(nameof(dmbProvider));
-			oldDmb.Dispose();
+			return oldDmb;
 		}
 
 		/// <inheritdoc />
@@ -639,9 +648,7 @@ namespace Tgstation.Server.Host.Components.Session
 						parsedChannels);
 					break;
 				case BridgeCommandType.Prime:
-					var oldPrimeTcs = primeTcs;
-					primeTcs = new TaskCompletionSource();
-					oldPrimeTcs.SetResult();
+					Interlocked.Exchange(ref primeTcs, new TaskCompletionSource()).SetResult();
 					break;
 				case BridgeCommandType.Kill:
 					Logger.LogInformation("Bridge requested process termination!");
@@ -722,6 +729,7 @@ namespace Tgstation.Server.Host.Components.Session
 
 					// Load custom commands
 					chatTrackingContext.CustomCommands = parameters.CustomCommands;
+					Interlocked.Exchange(ref startupTcs, new TaskCompletionSource()).SetResult();
 					break;
 				case BridgeCommandType.Reboot:
 					if (ClosePortOnReboot)
@@ -731,9 +739,7 @@ namespace Tgstation.Server.Host.Components.Session
 						portClosedForReboot = true;
 					}
 
-					var oldRebootTcs = rebootTcs;
-					rebootTcs = new TaskCompletionSource();
-					oldRebootTcs.SetResult();
+					Interlocked.Exchange(ref rebootTcs, new TaskCompletionSource()).SetResult();
 					break;
 				case BridgeCommandType.Chunk:
 					return await ProcessChunk<BridgeParameters, BridgeResponse>(ProcessBridgeCommand, BridgeError, parameters.Chunk, cancellationToken);

src/Tgstation.Server.Host/Components/Watchdog/BasicWatchdog.cs

@@ -170,6 +170,8 @@ namespace Tgstation.Server.Host.Components.Watchdog
 				case MonitorActivationReason.ActiveServerPrimed:
 					await HandleEvent(EventType.WorldPrime, Enumerable.Empty<string>(), false, cancellationToken);
 					break;
+				case MonitorActivationReason.ActiveServerStartup:
+					break; // unused in BasicWatchdog
 				case MonitorActivationReason.Heartbeat:
 				default:
 					throw new InvalidOperationException($"Invalid activation reason: {reason}");

src/Tgstation.Server.Host/Components/Watchdog/MonitorActivationReason.cs

@@ -34,5 +34,11 @@
 		/// Server primed.
 		/// </summary>
 		ActiveServerPrimed,
+
+		/// <summary>
+		/// Server started.
+		/// </summary>
+		/// <remarks>The monitor misses the first startup of a session.</remarks>
+		ActiveServerStartup,
 	}
 }

src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs

@@ -796,6 +796,7 @@ namespace Tgstation.Server.Host.Components.Watchdog
 				MonitorAction nextAction = MonitorAction.Continue;
 				Task activeServerLifetime = null,
 					activeServerReboot = null,
+					activeServerStartup = null,
 					serverPrimed = null,
 					activeLaunchParametersChanged = null,
 					newDmbAvailable = null;
@@ -825,12 +826,14 @@ namespace Tgstation.Server.Host.Components.Watchdog
 									TryUpdateTask(ref activeServerLifetime, () => controller.Lifetime);
 									TryUpdateTask(ref activeServerReboot, () => controller.OnReboot);
 									TryUpdateTask(ref serverPrimed, () => controller.OnPrime);
+									TryUpdateTask(ref activeServerStartup, () => controller.OnStartup);
 								}
 								else
 								{
 									activeServerLifetime = controller.Lifetime;
 									activeServerReboot = controller.OnReboot;
 									serverPrimed = controller.OnPrime;
+									activeServerStartup = controller.OnStartup;
 									lastController = controller;
 								}
 
@@ -862,6 +865,7 @@ namespace Tgstation.Server.Host.Components.Watchdog
 							var toWaitOn = Task.WhenAny(
 								activeServerLifetime,
 								activeServerReboot,
+								activeServerStartup,
 								heartbeat,
 								newDmbAvailable,
 								cancelTcs.Task,
@@ -908,7 +912,8 @@ namespace Tgstation.Server.Host.Components.Watchdog
 										|| CheckActivationReason(ref newDmbAvailable, MonitorActivationReason.NewDmbAvailable)
 										|| CheckActivationReason(ref activeLaunchParametersChanged, MonitorActivationReason.ActiveLaunchParametersUpdated)
 										|| CheckActivationReason(ref heartbeat, MonitorActivationReason.Heartbeat)
-										|| CheckActivationReason(ref serverPrimed, MonitorActivationReason.ActiveServerPrimed);
+										|| CheckActivationReason(ref serverPrimed, MonitorActivationReason.ActiveServerPrimed)
+										|| CheckActivationReason(ref activeServerStartup, MonitorActivationReason.ActiveServerStartup);
 
 									UpdateMonitoredTasks();
 

src/Tgstation.Server.Host/Components/Watchdog/WindowsWatchdog.cs

@@ -1,4 +1,6 @@
 using System;
+using System.Collections.Generic;
+using System.Diagnostics;
 using System.Threading;
 using System.Threading.Tasks;
 
@@ -37,11 +39,21 @@ namespace Tgstation.Server.Host.Components.Watchdog
 		/// </summary>
 		readonly ISymlinkFactory symlinkFactory;
 
+		/// <summary>
+		/// <see cref="List{T}"/> of <see cref="Task"/>s that are waiting to clean up old deployments.
+		/// </summary>
+		readonly List<Task> deploymentCleanupTasks;
+
 		/// <summary>
 		/// The active <see cref="SwappableDmbProvider"/> for <see cref="WatchdogBase.ActiveLaunchParameters"/>.
 		/// </summary>
 		SwappableDmbProvider pendingSwappable;
 
+		/// <summary>
+		/// The <see cref="TaskCompletionSource"/> representing the cleanup of an unused <see cref="IDmbProvider"/>.
+		/// </summary>
+		volatile TaskCompletionSource deploymentCleanupGate;
+
 		/// <summary>
 		/// Initializes a new instance of the <see cref="WindowsWatchdog"/> class.
 		/// </summary>
@@ -98,11 +110,16 @@ namespace Tgstation.Server.Host.Components.Watchdog
 			{
 				GameIOManager = gameIOManager ?? throw new ArgumentNullException(nameof(gameIOManager));
 				this.symlinkFactory = symlinkFactory ?? throw new ArgumentNullException(nameof(symlinkFactory));
+
+				deploymentCleanupTasks = new List<Task>();
 			}
 			catch
 			{
 				// Async dispose is for if we have controllers running, not the case here
-				DisposeAsync().AsTask().GetAwaiter().GetResult();
+				var disposeTask = DisposeAsync();
+				Debug.Assert(disposeTask.IsCompleted, "This should always be true during construction!");
+				disposeTask.GetAwaiter().GetResult();
+
 				throw;
 			}
 		}
@@ -116,6 +133,8 @@ namespace Tgstation.Server.Host.Components.Watchdog
 			ActiveSwappable = null;
 			pendingSwappable?.Dispose();
 			pendingSwappable = null;
+
+			await DrainDeploymentCleanupTasks(true);
 		}
 
 		/// <inheritdoc />
@@ -124,11 +143,55 @@ namespace Tgstation.Server.Host.Components.Watchdog
 			if (pendingSwappable != null)
 			{
 				var updateTask = BeforeApplyDmb(pendingSwappable.CompileJob, cancellationToken);
-				Logger.LogTrace("Replacing activeSwappable with pendingSwappable...");
+
 				if (!pendingSwappable.Swapped)
 					await PerformDmbSwap(pendingSwappable, cancellationToken);
 
-				Server.ReplaceDmbProvider(pendingSwappable);
+				var currentCompileJobId = Server.ReattachInformation.Dmb.CompileJob.Id;
+
+				await DrainDeploymentCleanupTasks(false);
+
+				IDisposable lingeringDeployment;
+				var localDeploymentCleanupGate = new TaskCompletionSource();
+				async Task CleanupLingeringDeployment()
+				{
+					var lingeringDeploymentExpirySeconds = ActiveLaunchParameters.StartupTimeout.Value;
+					Logger.LogDebug(
+						"Holding old deployment {compileJobId} for up to {expiry} seconds...",
+						currentCompileJobId,
+						lingeringDeploymentExpirySeconds);
+
+					var timeout = AsyncDelayer.Delay(TimeSpan.FromSeconds(lingeringDeploymentExpirySeconds), cancellationToken);
+
+					var completedTask = await Task.WhenAny(
+						localDeploymentCleanupGate.Task,
+						timeout);
+
+					var timedOut = completedTask == timeout;
+					Logger.Log(
+						timedOut
+							? LogLevel.Warning
+							: LogLevel.Trace,
+						"Releasing old deployment {compileJobId}{afterTimeout}",
+						timedOut
+							? " due to timeout!"
+							: "...");
+
+					lingeringDeployment.Dispose();
+				}
+
+				var oldDeploymentCleanupGate = Interlocked.Exchange(ref deploymentCleanupGate, localDeploymentCleanupGate);
+				oldDeploymentCleanupGate?.TrySetResult();
+
+				Logger.LogTrace("Replacing activeSwappable with pendingSwappable...");
+
+				lock (deploymentCleanupTasks)
+				{
+					lingeringDeployment = Server.ReplaceDmbProvider(pendingSwappable);
+					deploymentCleanupTasks.Add(
+						CleanupLingeringDeployment());
+				}
+
 				ActiveSwappable = pendingSwappable;
 				pendingSwappable = null;
 
@@ -240,6 +303,16 @@ namespace Tgstation.Server.Host.Components.Watchdog
 			await base.SessionStartupPersist(cancellationToken);
 		}
 
+		/// <inheritdoc />
+		protected override async Task<MonitorAction> HandleMonitorWakeup(MonitorActivationReason reason, CancellationToken cancellationToken)
+		{
+			var result = await base.HandleMonitorWakeup(reason, cancellationToken);
+			if (reason == MonitorActivationReason.ActiveServerStartup)
+				await DrainDeploymentCleanupTasks(false);
+
+			return result;
+		}
+
 		/// <summary>
 		/// Create the initial link to the live game directory using <see cref="ActiveSwappable"/>.
 		/// </summary>
@@ -284,5 +357,35 @@ namespace Tgstation.Server.Host.Components.Watchdog
 					server.Resume();
 			}
 		}
+
+		/// <summary>
+		/// Asynchronously drain <see cref="deploymentCleanupTasks"/>.
+		/// </summary>
+		/// <param name="blocking">If <see langword="true"/>, all <see cref="Task"/>s will be <see langword="await"/>ed. Otherwise, only <see cref="Task"/>s with <see cref="Task.IsCompleted"/> set will be <see langword="await"/>ed.</param>
+		/// <returns>A <see cref="Task"/> representing the running operation.</returns>
+		Task DrainDeploymentCleanupTasks(bool blocking)
+		{
+			Logger.LogTrace("DrainDeploymentCleanupTasks...");
+			var localDeploymentCleanupGate = Interlocked.Exchange(ref deploymentCleanupGate, null);
+			localDeploymentCleanupGate?.TrySetResult();
+
+			List<Task> localDeploymentCleanupTasks;
+			lock (deploymentCleanupTasks)
+			{
+				var totalActiveTasks = deploymentCleanupTasks.Count;
+				localDeploymentCleanupTasks = new List<Task>(totalActiveTasks);
+				for (var i = totalActiveTasks - 1; i >= 0; --i)
+				{
+					var currentTask = deploymentCleanupTasks[i];
+					if (!blocking && !currentTask.IsCompleted)
+						continue;
+
+					localDeploymentCleanupTasks.Add(currentTask);
+					deploymentCleanupTasks.RemoveAt(i);
+				}
+			}
+
+			return Task.WhenAll(localDeploymentCleanupTasks);
+		}
 	}
 }