From bab9d81aa0a5a74d2d479ae3c06f06bb9d02ee69 Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 5 Jun 2023 21:24:12 -0400 Subject: [PATCH 01/10] Fix attempting to wait on a job after the JobService has stopped --- .../Components/Instance.cs | 6 ++--- src/Tgstation.Server.Host/Jobs/JobHandler.cs | 25 +++++++++++-------- src/Tgstation.Server.Host/Jobs/JobService.cs | 10 ++++++++ 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Instance.cs b/src/Tgstation.Server.Host/Components/Instance.cs index 95ca246c42..0e441edb15 100644 --- a/src/Tgstation.Server.Host/Components/Instance.cs +++ b/src/Tgstation.Server.Host/Components/Instance.cs @@ -514,11 +514,9 @@ namespace Tgstation.Server.Host.Components await jobManager.RegisterOperation( repositoryUpdateJob, RepositoryAutoUpdateJob, - cancellationToken) - ; + cancellationToken); - // DCT: First token will cancel the job, second is for cancelling the cancellation, unwanted - await jobManager.WaitForJobCompletion(repositoryUpdateJob, null, cancellationToken, default); + await jobManager.WaitForJobCompletion(repositoryUpdateJob, null, cancellationToken, cancellationToken); Job compileProcessJob; using (var repo = await RepositoryManager.LoadRepository(cancellationToken)) diff --git a/src/Tgstation.Server.Host/Jobs/JobHandler.cs b/src/Tgstation.Server.Host/Jobs/JobHandler.cs index 19941fbefd..98cbe88651 100644 --- a/src/Tgstation.Server.Host/Jobs/JobHandler.cs +++ b/src/Tgstation.Server.Host/Jobs/JobHandler.cs @@ -11,6 +11,21 @@ namespace Tgstation.Server.Host.Jobs /// sealed class JobHandler : IDisposable { + /// + /// If the job has started. + /// + public bool Started => task != null; + + /// + /// The progress of the job. + /// + public int? Progress { get; set; } + + /// + /// The stage of the job. + /// + public string Stage { get; set; } + /// /// The for . /// @@ -39,16 +54,6 @@ namespace Tgstation.Server.Host.Jobs /// public void Dispose() => cancellationTokenSource.Dispose(); - /// - /// The progress of the job. - /// - public int? Progress { get; set; } - - /// - /// The stage of the job. - /// - public string Stage { get; set; } - /// /// Wait for to complete. /// diff --git a/src/Tgstation.Server.Host/Jobs/JobService.cs b/src/Tgstation.Server.Host/Jobs/JobService.cs index c0d01f2dcf..09350a4c4c 100644 --- a/src/Tgstation.Server.Host/Jobs/JobService.cs +++ b/src/Tgstation.Server.Host/Jobs/JobService.cs @@ -256,13 +256,23 @@ namespace Tgstation.Server.Host.Jobs { if (job == null) throw new ArgumentNullException(nameof(job)); + + if (!cancellationToken.CanBeCanceled) + throw new ArgumentException("A cancellable CancellationToken should be provided!", nameof(cancellationToken)); + JobHandler handler; + bool noMoreJobsShouldStart; lock (synchronizationLock) { if (!jobs.TryGetValue(job.Id.Value, out handler)) return; + + noMoreJobsShouldStart = this.noMoreJobsShouldStart; } + if (noMoreJobsShouldStart && !handler.Started) + await Extensions.TaskExtensions.InfiniteTask().WithToken(cancellationToken); + Task cancelTask = null; using (jobCancellationToken.Register(() => cancelTask = CancelJob(job, canceller, true, cancellationToken))) await handler.Wait(cancellationToken); From cca2e4a9b7de11360d30ddad89f069b02f303cf9 Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 5 Jun 2023 21:26:41 -0400 Subject: [PATCH 02/10] InfiniteTask from function to property --- .../Components/Watchdog/WatchdogBase.cs | 2 +- .../Extensions/TaskExtensions.cs | 11 +++++------ src/Tgstation.Server.Host/Jobs/JobService.cs | 2 +- src/Tgstation.Server.Host/Swarm/SwarmService.cs | 2 +- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs b/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs index 725f99fb3a..258ab9709c 100644 --- a/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs +++ b/src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs @@ -854,7 +854,7 @@ namespace Tgstation.Server.Host.Components.Watchdog var heartbeatSeconds = ActiveLaunchParameters.HeartbeatSeconds.Value; var heartbeat = heartbeatSeconds == 0 || !controller.DMApiAvailable - ? Extensions.TaskExtensions.InfiniteTask() + ? Extensions.TaskExtensions.InfiniteTask : Task.Delay( TimeSpan.FromSeconds(heartbeatSeconds), cancellationToken); diff --git a/src/Tgstation.Server.Host/Extensions/TaskExtensions.cs b/src/Tgstation.Server.Host/Extensions/TaskExtensions.cs index a8ee467d1e..bd65a629d9 100644 --- a/src/Tgstation.Server.Host/Extensions/TaskExtensions.cs +++ b/src/Tgstation.Server.Host/Extensions/TaskExtensions.cs @@ -14,6 +14,11 @@ namespace Tgstation.Server.Host.Extensions /// static readonly TaskCompletionSource InfiniteTaskCompletionSource = new (); + /// + /// Gets a that never completes. + /// + public static Task InfiniteTask => InfiniteTaskCompletionSource.Task; + /// /// Create a that can be awaited while respecting a given . /// @@ -53,11 +58,5 @@ namespace Tgstation.Server.Host.Extensions return await task; } - - /// - /// Creates a that never completes. - /// - /// A never ending . - public static Task InfiniteTask() => InfiniteTaskCompletionSource.Task; } } diff --git a/src/Tgstation.Server.Host/Jobs/JobService.cs b/src/Tgstation.Server.Host/Jobs/JobService.cs index 09350a4c4c..971150061a 100644 --- a/src/Tgstation.Server.Host/Jobs/JobService.cs +++ b/src/Tgstation.Server.Host/Jobs/JobService.cs @@ -271,7 +271,7 @@ namespace Tgstation.Server.Host.Jobs } if (noMoreJobsShouldStart && !handler.Started) - await Extensions.TaskExtensions.InfiniteTask().WithToken(cancellationToken); + await Extensions.TaskExtensions.InfiniteTask.WithToken(cancellationToken); Task cancelTask = null; using (jobCancellationToken.Register(() => cancelTask = CancelJob(job, canceller, true, cancellationToken))) diff --git a/src/Tgstation.Server.Host/Swarm/SwarmService.cs b/src/Tgstation.Server.Host/Swarm/SwarmService.cs index d53456c8ac..8f8a5ce7a9 100644 --- a/src/Tgstation.Server.Host/Swarm/SwarmService.cs +++ b/src/Tgstation.Server.Host/Swarm/SwarmService.cs @@ -380,7 +380,7 @@ namespace Tgstation.Server.Host.Swarm ? asyncDelayer.Delay( TimeSpan.FromMinutes(UpdateCommitTimeoutMinutes), cancellationToken) - : Extensions.TaskExtensions.InfiniteTask().WithToken(cancellationToken); + : Extensions.TaskExtensions.InfiniteTask.WithToken(cancellationToken); var commitTask = Task.WhenAny(commitTcsTask, timeoutTask); From e71a34a9191c0c02c45f54ee4fe9ccfb504f0947 Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 5 Jun 2023 21:27:52 -0400 Subject: [PATCH 03/10] Increase a test timeout --- tests/Tgstation.Server.Tests/Live/Instance/RepositoryTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Tgstation.Server.Tests/Live/Instance/RepositoryTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/RepositoryTest.cs index c35ccd70dd..05f3b90ec2 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/RepositoryTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/RepositoryTest.cs @@ -59,8 +59,8 @@ namespace Tgstation.Server.Tests.Live.Instance if (!targetActiveJob.Progress.HasValue) { - // give it 15 more seconds - targetActiveJob = await WaitForJobProgress(targetActiveJob, 15, cancellationToken); + // give it a few more seconds + targetActiveJob = await WaitForJobProgress(targetActiveJob, 30, cancellationToken); allJobs = await JobsClient.List(null, cancellationToken); } From 69a61b49308b18f8186a594635f2970d491f111d Mon Sep 17 00:00:00 2001 From: Dominion Date: Tue, 6 Jun 2023 08:05:10 -0400 Subject: [PATCH 04/10] Provide a valid CancellationToken --- src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs index f9ef3396c6..bfc541e6f9 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/Provider.cs @@ -309,7 +309,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers cancellationToken); // DCT: Always wait for the job to complete here - await jobManager.WaitForJobCompletion(job, null, cancellationToken, default); + await jobManager.WaitForJobCompletion(job, null, cancellationToken, cancellationToken); } } catch (OperationCanceledException e) From 257459e49242cf6f25ce3b29d63ec2de0a49a205 Mon Sep 17 00:00:00 2001 From: Dominion Date: Tue, 6 Jun 2023 08:06:20 -0400 Subject: [PATCH 05/10] My Discord name got migrated --- SECURITY.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SECURITY.md b/SECURITY.md index 5d79c18579..1e624e68bb 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -16,6 +16,6 @@ Vulnerabilities should ideally be reported by directly messaging one of the TGS Here is a list of their discord IDs. -@Cyberboss - Dominion#0444 (<@133295178197893120>) +@Cyberboss - dominion (<@133295178197893120>) Once reported, they will handle the processing of the security advisory. From ff62258507eb9e41fe7d8b3ffccc4f555de3f513 Mon Sep 17 00:00:00 2001 From: Ilya Shipitsin Date: Tue, 6 Jun 2023 23:19:10 +0200 Subject: [PATCH 06/10] CI: skip Doxygen builds on forks --- .github/workflows/ci-suite.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-suite.yml b/.github/workflows/ci-suite.yml index 7fd6134029..0bf8845e46 100644 --- a/.github/workflows/ci-suite.yml +++ b/.github/workflows/ci-suite.yml @@ -184,7 +184,7 @@ jobs: doxyfile-path: 'docs/Doxyfile' - name: gh-pages push - if: github.event_name == 'push' && github.event.ref == 'refs/heads/dev' + if: github.event_name == 'push' && github.event.ref == 'refs/heads/dev' && env.TGS_RELEASE_NOTES_TOKEN != '' run: | git clone -b gh-pages --single-branch "https://git@github.com/tgstation/tgstation-server" $HOME/tgsdox pushd $HOME/tgsdox From a9074b318616f1273484a4e67ac20c9b1106f52b Mon Sep 17 00:00:00 2001 From: Dominion Date: Tue, 6 Jun 2023 22:49:06 -0400 Subject: [PATCH 07/10] Starting to narrow down that spurious Live tests socket error --- tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs index 3b08f87812..7f251899ee 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs @@ -531,7 +531,7 @@ namespace Tgstation.Server.Tests.Live.Instance TopicResponse topicRequestResult = null; try { - System.Console.WriteLine($"Topic limit test S:{payloadSize}..."); + System.Console.WriteLine($"Topic send limit test S:{currentSize}..."); topicRequestResult = await TopicClientNoLogger.SendTopic( IPAddress.Loopback, $"tgs_integration_test_tactics3={TopicClient.SanitizeString(JsonConvert.SerializeObject(topic, DMApiConstants.SerializerSettings))}", @@ -563,6 +563,8 @@ namespace Tgstation.Server.Tests.Live.Instance Assert.AreEqual(DMApiConstants.MaximumTopicRequestLength, (uint)lastSize); + System.Console.WriteLine("TEST: Receiving Topic tests topics..."); + // Receive baseSize = 1; nextPow = 0; @@ -570,6 +572,7 @@ namespace Tgstation.Server.Tests.Live.Instance while (!cancellationToken.IsCancellationRequested) { var currentSize = baseSize + (int)Math.Pow(2, nextPow); + System.Console.WriteLine($"Topic send limit test S:{currentSize}..."); var topicRequestResult = await TopicClientNoLogger.SendTopic( IPAddress.Loopback, $"tgs_integration_test_tactics4={TopicClient.SanitizeString(currentSize.ToString())}", From eba904e700e92ecefb4be719a818bfe04fab4fd9 Mon Sep 17 00:00:00 2001 From: Dominion Date: Tue, 6 Jun 2023 23:03:58 -0400 Subject: [PATCH 08/10] Fix a log message and dump tests edge case --- .../Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs index 7f251899ee..5a781f50f8 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs @@ -246,7 +246,11 @@ namespace Tgstation.Server.Tests.Live.Instance jobTcs.SetResult(); await killTask; } - Assert.IsTrue(job.ErrorCode == ErrorCode.DreamDaemonOffline || job.ErrorCode == ErrorCode.GCoreFailure, $"{job.ErrorCode}: {job.ExceptionDetails}"); + + // this can also happen + if (!(new PlatformIdentifier().IsWindows && job.ExceptionDetails.Contains("BetterWin32Errors.Win32Exception: E_ACCESSDENIED: Access is denied."))) + Assert.IsTrue(job.ErrorCode == ErrorCode.DreamDaemonOffline || job.ErrorCode == ErrorCode.GCoreFailure, $"{job.ErrorCode}: {job.ExceptionDetails}"); + await Task.Delay(TimeSpan.FromSeconds(20), cancellationToken); var ddStatus = await instanceClient.DreamDaemon.Read(cancellationToken); @@ -572,7 +576,7 @@ namespace Tgstation.Server.Tests.Live.Instance while (!cancellationToken.IsCancellationRequested) { var currentSize = baseSize + (int)Math.Pow(2, nextPow); - System.Console.WriteLine($"Topic send limit test S:{currentSize}..."); + System.Console.WriteLine($"Topic recieve limit test S:{currentSize}..."); var topicRequestResult = await TopicClientNoLogger.SendTopic( IPAddress.Loopback, $"tgs_integration_test_tactics4={TopicClient.SanitizeString(currentSize.ToString())}", From d41715c512166adf881cfdbbd1e6a0be9b447ee4 Mon Sep 17 00:00:00 2001 From: Dominion Date: Tue, 6 Jun 2023 23:05:27 -0400 Subject: [PATCH 09/10] More dump tests edge cases --- tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs index 5a781f50f8..1be387c72a 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs @@ -247,8 +247,10 @@ namespace Tgstation.Server.Tests.Live.Instance await killTask; } - // this can also happen - if (!(new PlatformIdentifier().IsWindows && job.ExceptionDetails.Contains("BetterWin32Errors.Win32Exception: E_ACCESSDENIED: Access is denied."))) + // these can also happen + if (!(new PlatformIdentifier().IsWindows + && (job.ExceptionDetails.Contains("BetterWin32Errors.Win32Exception: E_ACCESSDENIED: Access is denied.") + || job.ExceptionDetails.Contains("BetterWin32Errors.Win32Exception: E_HANDLE: The handle is invalid.")))) Assert.IsTrue(job.ErrorCode == ErrorCode.DreamDaemonOffline || job.ErrorCode == ErrorCode.GCoreFailure, $"{job.ErrorCode}: {job.ExceptionDetails}"); await Task.Delay(TimeSpan.FromSeconds(20), cancellationToken); From e586102381c6153ec3c01e943925885e65258a98 Mon Sep 17 00:00:00 2001 From: Dominion Date: Tue, 6 Jun 2023 23:17:01 -0400 Subject: [PATCH 10/10] EVEN MORE DUMP TESTS EDGE CASES I fucking hate this test --- tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs index 1be387c72a..c97cac3308 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs @@ -250,7 +250,8 @@ namespace Tgstation.Server.Tests.Live.Instance // these can also happen if (!(new PlatformIdentifier().IsWindows && (job.ExceptionDetails.Contains("BetterWin32Errors.Win32Exception: E_ACCESSDENIED: Access is denied.") - || job.ExceptionDetails.Contains("BetterWin32Errors.Win32Exception: E_HANDLE: The handle is invalid.")))) + || job.ExceptionDetails.Contains("BetterWin32Errors.Win32Exception: E_HANDLE: The handle is invalid.") + || job.ExceptionDetails.Contains("BetterWin32Errors.Win32Exception: 3489660936: Unknown error (0xd0000008)")))) // kek Assert.IsTrue(job.ErrorCode == ErrorCode.DreamDaemonOffline || job.ErrorCode == ErrorCode.GCoreFailure, $"{job.ErrorCode}: {job.ExceptionDetails}"); await Task.Delay(TimeSpan.FromSeconds(20), cancellationToken);