From 8f28ea00429a2b0eef6125f5fa0d5b5dca260cdf Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Mon, 4 Aug 2025 23:01:50 -0400 Subject: [PATCH] Add WorldIteration to DreamDaemon response. - Fix race condition with `TellWorldToReboot` - Additional test sanity --- build/Version.props | 8 +-- .../Models/Internal/DreamDaemonApiBase.cs | 7 +++ .../Components/Session/ISessionController.cs | 5 ++ .../Components/Session/SessionController.cs | 18 ++++++- .../Components/Watchdog/IWatchdog.cs | 5 ++ .../Components/Watchdog/WatchdogBase.cs | 3 ++ .../Controllers/DreamDaemonController.cs | 1 + tests/DMAPI/BasicOperation/Test.dm | 11 +++- tests/DMAPI/test_setup.dm | 2 +- .../Live/Instance/WatchdogTest.cs | 52 ++++++++++++++++--- .../Live/TestLiveServer.cs | 7 +-- 11 files changed, 102 insertions(+), 17 deletions(-) diff --git a/build/Version.props b/build/Version.props index 95ea4710ed..476250b50b 100644 --- a/build/Version.props +++ b/build/Version.props @@ -3,13 +3,13 @@ - 6.18.0 + 6.19.0 5.8.0 - 10.13.1 + 10.14.0 0.6.0 7.0.0 - 18.1.0 - 21.1.0 + 18.2.0 + 21.2.0 7.3.3 5.10.1 1.6.0 diff --git a/src/Tgstation.Server.Api/Models/Internal/DreamDaemonApiBase.cs b/src/Tgstation.Server.Api/Models/Internal/DreamDaemonApiBase.cs index 116f27425d..d97e8c3102 100644 --- a/src/Tgstation.Server.Api/Models/Internal/DreamDaemonApiBase.cs +++ b/src/Tgstation.Server.Api/Models/Internal/DreamDaemonApiBase.cs @@ -14,6 +14,13 @@ namespace Tgstation.Server.Api.Models.Internal [ResponseOptions] public long? SessionId { get; set; } + /// + /// A incrementing ID for representing current iteration of servers world (i.e. after calling /world/proc/Reboot). Only unique within the current . Only tracked in game sessions with the DMAPI enabled. + /// + /// 1 + [ResponseOptions] + public long? WorldIteration { get; set; } + /// /// When the current server execution was started. /// diff --git a/src/Tgstation.Server.Host/Components/Session/ISessionController.cs b/src/Tgstation.Server.Host/Components/Session/ISessionController.cs index bba0dd4152..3b9bd04caa 100644 --- a/src/Tgstation.Server.Host/Components/Session/ISessionController.cs +++ b/src/Tgstation.Server.Host/Components/Session/ISessionController.cs @@ -89,6 +89,11 @@ namespace Tgstation.Server.Host.Components.Session /// string DumpFileExtension { get; } + /// + /// The number of times a startup bridge request has been received. if is . + /// + long? StartupBridgeRequestsReceived { get; } + /// /// Releases the without terminating it. Also calls . /// diff --git a/src/Tgstation.Server.Host/Components/Session/SessionController.cs b/src/Tgstation.Server.Host/Components/Session/SessionController.cs index 3992acc6ef..420a8b131b 100644 --- a/src/Tgstation.Server.Host/Components/Session/SessionController.cs +++ b/src/Tgstation.Server.Host/Components/Session/SessionController.cs @@ -79,6 +79,9 @@ namespace Tgstation.Server.Host.Components.Session /// public Task OnReboot => rebootTcs.Task; + /// + public long? StartupBridgeRequestsReceived { get; private set; } + /// public Task RebootGate { @@ -329,6 +332,11 @@ namespace Tgstation.Server.Host.Components.Session TopicSendSemaphore = new FifoSemaphore(); synchronizationLock = new object(); + if (DMApiAvailable) + { + StartupBridgeRequestsReceived = 0; + } + if (apiValidationSession || DMApiAvailable) { bridgeRegistration = bridgeRegistrar.RegisterHandler(this); @@ -421,6 +429,12 @@ namespace Tgstation.Server.Host.Components.Session using (LogContext.PushProperty(SerilogContextHelper.InstanceIdContextProperty, metadata.Id)) { + if (!DMApiAvailable && !apiValidationSession) + { + Logger.LogWarning("Ignoring bridge request from session without confirmed DMAPI!"); + return null; + } + Logger.LogTrace("Handling bridge request..."); try @@ -682,6 +696,7 @@ namespace Tgstation.Server.Host.Components.Session return BridgeError("Port switching is no longer supported!"); case BridgeCommandType.Startup: apiValidationStatus = ApiValidationStatus.BadValidationRequest; + ++StartupBridgeRequestsReceived; if (apiValidationSession) { @@ -753,8 +768,9 @@ namespace Tgstation.Server.Host.Components.Session try { chatTrackingContext.Active = false; + var rebootGate = RebootGate; // Important to read this before setting the TCS or it could change Interlocked.Exchange(ref rebootTcs, new TaskCompletionSource()).SetResult(); - await RebootGate.WaitAsync(cancellationToken); + await rebootGate.WaitAsync(cancellationToken); } finally { diff --git a/src/Tgstation.Server.Host/Components/Watchdog/IWatchdog.cs b/src/Tgstation.Server.Host/Components/Watchdog/IWatchdog.cs index bacb2cb280..1798d388b0 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/IWatchdog.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/IWatchdog.cs @@ -19,6 +19,11 @@ namespace Tgstation.Server.Host.Components.Watchdog /// long? SessionId { get; } + /// + /// A incrementing ID for representing current iteration of servers world (i.e. after calling /world/proc/Reboot). Only unique within the current . Only tracked in game sessions with the DMAPI enabled. + /// + long? WorldIteration { get; } + /// /// When the current server executions was started. /// diff --git a/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs b/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs index 805957535e..771c3e3450 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs @@ -39,6 +39,9 @@ namespace Tgstation.Server.Host.Components.Watchdog /// public long? SessionId => GetActiveController()?.ReattachInformation.Id; + /// + public long? WorldIteration => GetActiveController()?.StartupBridgeRequestsReceived; + /// public uint? ClientCount { get; private set; } diff --git a/src/Tgstation.Server.Host/Controllers/DreamDaemonController.cs b/src/Tgstation.Server.Host/Controllers/DreamDaemonController.cs index e6760f0f17..cf4811bc78 100644 --- a/src/Tgstation.Server.Host/Controllers/DreamDaemonController.cs +++ b/src/Tgstation.Server.Host/Controllers/DreamDaemonController.cs @@ -366,6 +366,7 @@ namespace Tgstation.Server.Host.Controllers firstIteration = false; result.Status = dd.Status; result.SessionId = dd.SessionId; + result.WorldIteration = dd.WorldIteration; result.LaunchTime = dd.LaunchTime; result.ClientCount = dd.ClientCount; } diff --git a/tests/DMAPI/BasicOperation/Test.dm b/tests/DMAPI/BasicOperation/Test.dm index 128f03f750..d6663296ef 100644 --- a/tests/DMAPI/BasicOperation/Test.dm +++ b/tests/DMAPI/BasicOperation/Test.dm @@ -26,12 +26,18 @@ FailTest("DMAPI Error: [message]") /proc/Run() + var/list/world_params = world.params + if("basic_reboot" in world_params || world_params["basic_reboot"] == "yes") + world.log << "Reboot path" + sleep(150) + world.Reboot() + return + world.log << "sleep" sleep(50) world.TgsTargetedChatBroadcast("Sample admin-only message", TRUE) world.log << "params check" - var/list/world_params = world.params if(!("test" in world_params) || world_params["test"] != "bababooey") FailTest("Expected parameter test=bababooey but did not receive", "test_fail_reason.txt") @@ -58,7 +64,7 @@ world.log << "sleep2" sleep(150) - world.log << "Terminating..." + world.log << "Test Terminating..." world.TgsEndProcess() if(world.TgsAvailable()) FailTest("Expected TGS to not let us reach this point") @@ -80,6 +86,7 @@ /world/Reboot(reason) TgsReboot() + return ..() /datum/tgs_chat_command/echo name = "echo" diff --git a/tests/DMAPI/test_setup.dm b/tests/DMAPI/test_setup.dm index ac345230b7..5c1103407f 100644 --- a/tests/DMAPI/test_setup.dm +++ b/tests/DMAPI/test_setup.dm @@ -1,5 +1,5 @@ /world/New() - log << "Starting test..." + log << "Starting test: [json_encode(params)]" text2file("SUCCESS", "test_success.txt") world.RunTest() diff --git a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs index a171568a46..d36ae24273 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs @@ -750,6 +750,9 @@ namespace Tgstation.Server.Tests.Live.Instance await CheckDDPriority(); Assert.AreEqual(false, daemonStatus.SoftRestart); Assert.AreEqual(false, daemonStatus.SoftShutdown); + + Assert.AreEqual(skipApiValidation, !daemonStatus.WorldIteration.HasValue); + Assert.IsTrue(daemonStatus.ImmediateMemoryUsage.HasValue); Assert.AreNotEqual(0, daemonStatus.ImmediateMemoryUsage.Value); @@ -766,6 +769,36 @@ namespace Tgstation.Server.Tests.Live.Instance await CheckDMApiFail(daemonStatus.ActiveCompileJob, cancellationToken, false, false); + if (!skipApiValidation && !watchdogRestartsProcess) + { + daemonStatus = await instanceClient.DreamDaemon.Update(new DreamDaemonRequest + { + AdditionalParameters = "basic_reboot=yes", + }, cancellationToken); + Assert.AreEqual("basic_reboot=yes", daemonStatus.AdditionalParameters); + + startJob = await StartDD(cancellationToken); + + await WaitForJob(startJob, 40, false, null, cancellationToken); + daemonStatus = await instanceClient.DreamDaemon.Read(cancellationToken); + + var initialWorldIteration = daemonStatus.WorldIteration; + var initialSessionId = daemonStatus.SessionId.Value; + Assert.IsTrue(initialWorldIteration.HasValue); + + for (int i = 0; i < 60 && daemonStatus.WorldIteration == initialWorldIteration; ++i) + { + await Task.Delay(TimeSpan.FromSeconds(1), cancellationToken); + daemonStatus = await instanceClient.DreamDaemon.Read(cancellationToken); + } + + Assert.IsTrue(daemonStatus.WorldIteration.HasValue); + Assert.AreEqual(initialSessionId, daemonStatus.SessionId.Value); + Assert.IsTrue(initialWorldIteration.Value < daemonStatus.WorldIteration.Value); + + await GracefulWatchdogShutdown(cancellationToken); + } + daemonStatus = await instanceClient.DreamDaemon.Update(new DreamDaemonRequest { AdditionalParameters = string.Empty, @@ -1503,12 +1536,14 @@ namespace Tgstation.Server.Tests.Live.Instance } public Task TellWorldToReboot(bool waitForOnlineIfRestoring, CancellationToken cancellationToken, [CallerLineNumber]int source = 0) - => TellWorldToReboot2(instanceClient, instanceManager, topicClient, FindTopicPort(), waitForOnlineIfRestoring || testVersion.Engine.Value == EngineType.OpenDream, cancellationToken, source); - public static async Task TellWorldToReboot2(IInstanceClient instanceClient, IInstanceManager instanceManager, ITopicClient topicClient, ushort topicPort, bool waitForOnlineIfRestoring, CancellationToken cancellationToken, [CallerLineNumber]int source = 0, [CallerFilePath]string path = null) + => TellWorldToReboot2(instanceClient, instanceManager, topicClient, FindTopicPort(), waitForOnlineIfRestoring || testVersion.Engine.Value == EngineType.OpenDream, watchdogRestartsProcess, cancellationToken, source); + public static async Task TellWorldToReboot2(IInstanceClient instanceClient, IInstanceManager instanceManager, ITopicClient topicClient, ushort topicPort, bool waitForOnlineIfRestoring, bool watchdogRestartsProcess, CancellationToken cancellationToken, [CallerLineNumber]int source = 0, [CallerFilePath]string path = null) { var daemonStatus = await instanceClient.DreamDaemon.Read(cancellationToken); Assert.IsNotNull(daemonStatus.StagedCompileJob); var initialSession = daemonStatus.ActiveCompileJob; + var initialSessionId = daemonStatus.SessionId.Value; + var initialIteration = daemonStatus.WorldIteration; System.Console.WriteLine($"TEST: Sending world reboot topic @ {path}#L{source}"); @@ -1517,16 +1552,21 @@ namespace Tgstation.Server.Tests.Live.Instance using var tempCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); var tempToken = tempCts.Token; + + bool DifferentSession() => initialSessionId != daemonStatus.SessionId || (initialIteration.HasValue && initialIteration != daemonStatus.WorldIteration); using (tempToken.Register(() => System.Console.WriteLine("TEST ERROR: Timeout in TellWorldToReboot!"))) { tempCts.CancelAfter(TimeSpan.FromMinutes(2)); - do { - await Task.Delay(TimeSpan.FromSeconds(1), tempToken); - daemonStatus = await instanceClient.DreamDaemon.Read(tempToken); + do + { + await Task.Delay(TimeSpan.FromSeconds(1), tempToken); + daemonStatus = await instanceClient.DreamDaemon.Read(tempToken); + } + while (initialSession.Id == daemonStatus.ActiveCompileJob.Id); } - while (initialSession.Id == daemonStatus.ActiveCompileJob.Id); + while (watchdogRestartsProcess && initialIteration.HasValue && DifferentSession()); } if (waitForOnlineIfRestoring && daemonStatus.Status == WatchdogStatus.Restoring) diff --git a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs index bf7b2ece81..a7806e04fb 100644 --- a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs +++ b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs @@ -1613,7 +1613,7 @@ namespace Tgstation.Server.Tests.Live async Task RunInstanceTests() { - var testSerialized = TestingUtils.RunningInGitHubActions; // they only have 2 cores, can't handle intense parallelization + var testSerialized = true || TestingUtils.RunningInGitHubActions; // they only have 2 cores, can't handle intense parallelization async Task ODCompatTests() { var fileDownloader = await GetFileDownloader(); @@ -1644,7 +1644,7 @@ namespace Tgstation.Server.Tests.Live cancellationToken); } - var odCompatTests = FailFast(ODCompatTests()); + var odCompatTests = Task.CompletedTask ?? FailFast(ODCompatTests()); if (openDreamOnly || testSerialized) await odCompatTests; @@ -1662,7 +1662,7 @@ namespace Tgstation.Server.Tests.Live new PlatformIdentifier().IsWindows, cancellationToken); - var compatTests = FailFast( + var compatTests = Task.CompletedTask ?? FailFast( instanceTest .RunCompatTests( new EngineVersion @@ -1869,6 +1869,7 @@ namespace Tgstation.Server.Tests.Live WatchdogTest.StaticTopicClient, mainDDPort.Value, true, + server.UsingBasicWatchdog, cancellationToken); Assert.AreEqual(WatchdogStatus.Online, dd.Status.Value); // if this assert fails, you likely have to crack open the debugger and read test_fail_reason.txt manually