From 7b458a67da0016e8075ec1d646c9eae58ce2a297 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 23 Dec 2023 23:55:25 -0500 Subject: [PATCH] Nullify `WatchdogBase` --- .../Components/Watchdog/AdvancedWatchdog.cs | 4 +- .../Components/Watchdog/BasicWatchdog.cs | 2 +- .../Components/Watchdog/WatchdogBase.cs | 131 +++++++++++------- 3 files changed, 84 insertions(+), 53 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Watchdog/AdvancedWatchdog.cs b/src/Tgstation.Server.Host/Components/Watchdog/AdvancedWatchdog.cs index 347048921d..884541e9fb 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/AdvancedWatchdog.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/AdvancedWatchdog.cs @@ -233,7 +233,7 @@ namespace Tgstation.Server.Host.Components.Watchdog IDmbProvider compileJobProvider = DmbFactory.LockNextDmb(1); bool canSeamlesslySwap = CanUseSwappableDmbProvider(compileJobProvider); if (canSeamlesslySwap) - if (compileJobProvider.CompileJob.EngineVersion != ActiveCompileJob.EngineVersion) + if (compileJobProvider.CompileJob.EngineVersion != ActiveCompileJob!.EngineVersion) { // have to do a graceful restart Logger.LogDebug( @@ -265,7 +265,7 @@ namespace Tgstation.Server.Host.Components.Watchdog try { swappableProvider = CreateSwappableDmbProvider(compileJobProvider); - if (ActiveCompileJob.DMApiVersion == null) + if (ActiveCompileJob!.DMApiVersion == null) { Logger.LogWarning("Active compile job has no DMAPI! Commencing immediate .dmb swap. Note this behavior is known to be buggy in some DM code contexts. See https://github.com/tgstation/tgstation-server/issues/1550"); await PerformDmbSwap(swappableProvider, cancellationToken); diff --git a/src/Tgstation.Server.Host/Components/Watchdog/BasicWatchdog.cs b/src/Tgstation.Server.Host/Components/Watchdog/BasicWatchdog.cs index 37b9bed41c..10b6a7b19c 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/BasicWatchdog.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/BasicWatchdog.cs @@ -208,7 +208,7 @@ namespace Tgstation.Server.Host.Components.Watchdog /// protected override async ValueTask InitController( ValueTask eventTask, - ReattachInformation reattachInfo, + ReattachInformation? reattachInfo, CancellationToken cancellationToken) { // don't need a new dmb if reattaching diff --git a/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs b/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs index 0750f006d6..e922322995 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs @@ -27,8 +27,6 @@ using Tgstation.Server.Host.IO; using Tgstation.Server.Host.Jobs; using Tgstation.Server.Host.Utils; -#nullable disable - namespace Tgstation.Server.Host.Components.Watchdog { /// @@ -58,10 +56,10 @@ namespace Tgstation.Server.Host.Components.Watchdog public DreamDaemonLaunchParameters ActiveLaunchParameters { get; protected set; } /// - public DreamDaemonLaunchParameters LastLaunchParameters { get; protected set; } + public DreamDaemonLaunchParameters? LastLaunchParameters { get; protected set; } /// - public Models.CompileJob ActiveCompileJob => GetActiveController()?.CompileJob; + public Models.CompileJob? ActiveCompileJob => GetActiveController()?.CompileJob; /// public abstract RebootState? RebootState { get; } @@ -154,12 +152,12 @@ namespace Tgstation.Server.Host.Components.Watchdog /// /// The for the monitor loop. /// - CancellationTokenSource monitorCts; + CancellationTokenSource? monitorCts; /// /// The running the monitor loop. /// - Task monitorTask; + Task? monitorTask; /// /// Backing field for . @@ -395,7 +393,7 @@ namespace Tgstation.Server.Host.Components.Watchdog job, async (core, databaseContextFactory, paramJob, progressFunction, ct) => { - if (core.Watchdog != this) + if (core?.Watchdog != this) throw new InvalidOperationException(Instance.DifferentCoreExceptionMessage); using (await SemaphoreSlimContext.Lock(synchronizationSemaphore, ct)) @@ -418,7 +416,7 @@ namespace Tgstation.Server.Host.Components.Watchdog } /// - public async ValueTask HandleRestart(Version updateVersion, bool handlerMayDelayShutdownWithExtremelyLongRunningTasks, CancellationToken cancellationToken) + public async ValueTask HandleRestart(Version? updateVersion, bool handlerMayDelayShutdownWithExtremelyLongRunningTasks, CancellationToken cancellationToken) { if (handlerMayDelayShutdownWithExtremelyLongRunningTasks) { @@ -427,7 +425,7 @@ namespace Tgstation.Server.Host.Components.Watchdog if (Status != WatchdogStatus.Offline) { Logger.LogDebug("Waiting for server to gracefully shut down."); - await monitorTask.WaitAsync(cancellationToken); + await monitorTask!.WaitAsync(cancellationToken); } else Logger.LogTrace("Graceful shutdown requested but server is already offline."); @@ -490,7 +488,7 @@ namespace Tgstation.Server.Host.Components.Watchdog } /// - async ValueTask IEventConsumer.HandleEvent(EventType eventType, IEnumerable parameters, bool deploymentPipeline, CancellationToken cancellationToken) + async ValueTask IEventConsumer.HandleEvent(EventType eventType, IEnumerable parameters, bool deploymentPipeline, CancellationToken cancellationToken) { ArgumentNullException.ThrowIfNull(parameters); @@ -516,7 +514,7 @@ namespace Tgstation.Server.Host.Components.Watchdog /// to use, if any. /// The for the operation. /// A representing the running operation. - protected abstract ValueTask InitController(ValueTask eventTask, ReattachInformation reattachInfo, CancellationToken cancellationToken); + protected abstract ValueTask InitController(ValueTask eventTask, ReattachInformation? reattachInfo, CancellationToken cancellationToken); /// /// Launches the watchdog. @@ -531,7 +529,7 @@ namespace Tgstation.Server.Host.Components.Watchdog bool startMonitor, bool announce, bool announceFailure, - ReattachInformation reattachInfo, + ReattachInformation? reattachInfo, CancellationToken cancellationToken) { Logger.LogTrace("Begin LaunchImplNoLock"); @@ -612,7 +610,7 @@ namespace Tgstation.Server.Host.Components.Watchdog if (monitorTask == null) return false; var wasRunning = !monitorTask.IsCompleted; - monitorCts.Cancel(); + monitorCts!.Cancel(); await monitorTask; Logger.LogTrace("Stopped Monitor"); monitorCts.Dispose(); @@ -640,7 +638,7 @@ namespace Tgstation.Server.Host.Components.Watchdog if (!launchResult.StartupTime.HasValue) throw new JobException( ErrorCode.WatchdogStartupTimeout, - new JobException($"{serverName} timed out on startup: {ActiveLaunchParameters.StartupTimeout.Value}s")); + new JobException($"{serverName} timed out on startup: {ActiveLaunchParameters.StartupTimeout!.Value}s")); } /// @@ -685,8 +683,8 @@ namespace Tgstation.Server.Host.Components.Watchdog /// /// Get the active . /// - /// The active . - protected abstract ISessionController GetActiveController(); + /// The active , if any. + protected abstract ISessionController? GetActiveController(); /// /// Handles the actions to take when the monitor has to "wake up". @@ -718,9 +716,9 @@ namespace Tgstation.Server.Host.Components.Watchdog var eventTask = eventConsumer.HandleEvent( EventType.DeploymentActivation, - new List + new List { - GameIOManager.ResolvePath(newCompileJob.DirectoryName.ToString()), + GameIOManager.ResolvePath(newCompileJob.DirectoryName!.Value.ToString()), }, false, cancellationToken); @@ -845,13 +843,14 @@ namespace Tgstation.Server.Host.Components.Watchdog try { MonitorAction nextAction = MonitorAction.Continue; - Task activeServerLifetime = null, + Task? activeServerLifetime = null, activeServerReboot = null, activeServerStartup = null, serverPrimed = null, activeLaunchParametersChanged = null, - newDmbAvailable = null; - ISessionController lastController = null; + newDmbAvailable = null, + healthCheck = null; + ISessionController? lastController = null; var ranInitialDmbCheck = false; for (ulong iteration = 1; nextAction != MonitorAction.Exit; ++iteration) using (LogContext.PushProperty(SerilogContextHelper.WatchdogMonitorIterationContextProperty, iteration)) @@ -867,7 +866,7 @@ namespace Tgstation.Server.Host.Components.Watchdog void UpdateMonitoredTasks() { var sameController = lastController == controller; - void TryUpdateTask(ref Task oldTask, Func newTaskFactory) + void TryUpdateTask(ref Task? oldTask, Func newTaskFactory) { if (sameController && oldTask?.IsCompleted == true) return; @@ -875,7 +874,7 @@ namespace Tgstation.Server.Host.Components.Watchdog oldTask = newTaskFactory(); } - controller.RebootGate = nextMonitorWakeupTcs.Task; + controller!.RebootGate = nextMonitorWakeupTcs.Task; TryUpdateTask(ref activeServerLifetime, () => controller.Lifetime); TryUpdateTask(ref activeServerReboot, () => controller.OnReboot); @@ -898,28 +897,36 @@ namespace Tgstation.Server.Host.Components.Watchdog }); } - UpdateMonitoredTasks(); + if (controller != null) + { + UpdateMonitoredTasks(); - var healthCheckSeconds = ActiveLaunchParameters.HealthCheckSeconds.Value; - var healthCheck = healthCheckSeconds == 0 - || !controller.DMApiAvailable - ? Extensions.TaskExtensions.InfiniteTask - : Task.Delay( - TimeSpan.FromSeconds(healthCheckSeconds), - cancellationToken); + var healthCheckSeconds = ActiveLaunchParameters.HealthCheckSeconds!.Value; + healthCheck = healthCheckSeconds == 0 + || !controller.DMApiAvailable + ? Extensions.TaskExtensions.InfiniteTask + : Task.Delay( + TimeSpan.FromSeconds(healthCheckSeconds), + cancellationToken); - // cancel waiting if requested - var toWaitOn = Task.WhenAny( - activeServerLifetime, - activeServerReboot, - activeServerStartup, - healthCheck, - newDmbAvailable, - activeLaunchParametersChanged, - serverPrimed); + // cancel waiting if requested + var toWaitOn = Task.WhenAny( + activeServerLifetime!, + activeServerReboot!, + activeServerStartup!, + healthCheck, + newDmbAvailable!, + activeLaunchParametersChanged!, + serverPrimed!); - // wait for something to happen - await toWaitOn.WaitAsync(cancellationToken); + // wait for something to happen + await toWaitOn.WaitAsync(cancellationToken); + } + else + { + Logger.LogError("Controller was null on monitor wakeup! Attempting restart..."); + nextAction = MonitorAction.Restart; // excuse me wtf? + } cancellationToken.ThrowIfCancellationRequested(); Logger.LogTrace("Monitor activated"); @@ -928,7 +935,7 @@ namespace Tgstation.Server.Host.Components.Watchdog using (await SemaphoreSlimContext.Lock(synchronizationSemaphore, cancellationToken)) { // Set this sooner so chat sends don't hold us up - if (activeServerLifetime.IsCompleted) + if (activeServerLifetime!.IsCompleted) Status = WatchdogStatus.Restoring; // multiple things may have happened, handle them one at a time @@ -936,7 +943,7 @@ namespace Tgstation.Server.Host.Components.Watchdog { MonitorActivationReason activationReason = default; // this will always be assigned before being used - bool CheckActivationReason(ref Task task, MonitorActivationReason testActivationReason) + bool CheckActivationReason(ref Task? task, MonitorActivationReason testActivationReason) { var taskCompleted = task?.IsCompleted == true; task = null; @@ -1027,7 +1034,10 @@ namespace Tgstation.Server.Host.Components.Watchdog { Logger.LogTrace("Detaching server..."); var controller = GetActiveController(); - await controller.Release(); + if (controller != null) + await controller.Release(); + else + Logger.LogError("Controller was null on monitor shutdown!"); } } @@ -1091,6 +1101,9 @@ namespace Tgstation.Server.Host.Components.Watchdog { Logger.LogTrace("Sending health check to active server..."); var activeServer = GetActiveController(); + if (activeServer == null) + return MonitorAction.Restart; // uhhhh??? + var response = await activeServer.SendCommand(new TopicParameters(), cancellationToken); var shouldShutdown = activeServer.RebootState == Session.RebootState.Shutdown; @@ -1130,7 +1143,7 @@ namespace Tgstation.Server.Host.Components.Watchdog actionTaken, StringComparison.Ordinal)); - if (ActiveLaunchParameters.DumpOnHealthCheckRestart.Value) + if (ActiveLaunchParameters.DumpOnHealthCheckRestart!.Value) { Logger.LogDebug("DumpOnHealthCheckRestart enabled."); try @@ -1166,13 +1179,30 @@ namespace Tgstation.Server.Host.Components.Watchdog /// Handle any in a given topic . /// /// The . - void HandleChatResponses(TopicResponse result) + void HandleChatResponses(TopicResponse? result) { if (result?.ChatResponses != null) - foreach (var response in result.ChatResponses) + { + var warnedMissingChannelIds = false; + foreach (var response in result.ChatResponses + .Where(response => + { + if (response.ChannelIds == null) + { + if (!warnedMissingChannelIds) + { + Logger.LogWarning("DMAPI response contains null channelIds!"); + warnedMissingChannelIds = true; + } + + return false; + } + + return true; + })) Chat.QueueMessage( response, - response.ChannelIds + response.ChannelIds! .Select(channelIdString => { if (UInt64.TryParse(channelIdString, out var channelId)) @@ -1183,7 +1213,8 @@ namespace Tgstation.Server.Host.Components.Watchdog return null; }) .Where(nullableChannelId => nullableChannelId.HasValue) - .Select(nullableChannelId => nullableChannelId.Value)); + .Select(nullableChannelId => nullableChannelId!.Value)); + } } ///