From 786d76ea05155dc8bf5393d09db5357b2c2ad93d Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 29 May 2023 09:55:22 -0400 Subject: [PATCH 1/8] Fix race condition in test --- tests/Tgstation.Server.Tests/Live/TestLiveServer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs index b3a7596c5f..931cfa8594 100644 --- a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs +++ b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs @@ -855,8 +855,8 @@ namespace Tgstation.Server.Tests.Live var adminTest = FailFast(new AdministrationTest(adminClient.Administration).Run(cancellationToken)); var usersTest = FailFast(new UsersTest(adminClient).Run(cancellationToken)); var instanceMangagerTest = new InstanceManagerTest(adminClient, server.Directory); - var instancesTest = FailFast(instanceMangagerTest.RunPreTest(cancellationToken)); instance = await instanceMangagerTest.CreateTestInstance(cancellationToken); + var instancesTest = FailFast(instanceMangagerTest.RunPreTest(cancellationToken)); Assert.IsTrue(Directory.Exists(instance.Path)); var instanceClient = adminClient.Instances.CreateClient(instance); From 10fd6168a1f9f4564476788e6726e862a35b462d Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 29 May 2023 10:16:28 -0400 Subject: [PATCH 2/8] Fix a race condition in JobManager shutdown --- src/Tgstation.Server.Host/Jobs/JobManager.cs | 39 +++++++++++++++----- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/src/Tgstation.Server.Host/Jobs/JobManager.cs b/src/Tgstation.Server.Host/Jobs/JobManager.cs index 88828baca2..cac6b4fab3 100644 --- a/src/Tgstation.Server.Host/Jobs/JobManager.cs +++ b/src/Tgstation.Server.Host/Jobs/JobManager.cs @@ -54,6 +54,11 @@ namespace Tgstation.Server.Host.Jobs /// readonly object addCancelLock; + /// + /// Prevents jobs that are registered after shutdown from activating. + /// + volatile bool noMoreJobsShouldStart; + /// /// Initializes a new instance of the class. /// @@ -122,10 +127,15 @@ namespace Tgstation.Server.Host.Jobs { lock (addCancelLock) { + bool jobShouldStart; lock (synchronizationLock) + { jobs.Add(job.Id.Value, jobHandler); + jobShouldStart = !noMoreJobsShouldStart; + } - jobHandler.Start(); + if (jobShouldStart) + jobHandler.Start(); } } catch @@ -145,8 +155,7 @@ namespace Tgstation.Server.Host.Jobs .AsQueryable() .Where(y => !y.StoppedAt.HasValue) .Select(y => y.Id) - .ToListAsync(cancellationToken) - ; + .ToListAsync(cancellationToken); if (badJobIds.Count > 0) { logger.LogTrace("Cleaning {unfinishedJobCount} unfinished jobs...", badJobIds.Count); @@ -160,19 +169,29 @@ namespace Tgstation.Server.Host.Jobs await databaseContext.Save(cancellationToken); } + + noMoreJobsShouldStart = false; }); /// public async Task StopAsync(CancellationToken cancellationToken) { - var joinTasks = jobs.Select(x => CancelJob( - new Job + List> joinTasks; + lock (addCancelLock) + lock (synchronizationLock) { - Id = x.Key, - }, - null, - true, - cancellationToken)); + noMoreJobsShouldStart = true; + joinTasks = jobs.Select(x => CancelJob( + new Job + { + Id = x.Key, + }, + null, + true, + cancellationToken)) + .ToList(); + } + await Task.WhenAll(joinTasks); } From 79afd454c32d8b10b5b12695dadc23758ca5880a Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 29 May 2023 10:42:28 -0400 Subject: [PATCH 3/8] Fix tests not failing correctly --- tests/Tgstation.Server.Tests/Live/TestLiveServer.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs index 931cfa8594..1ade1e1c5a 100644 --- a/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs +++ b/tests/Tgstation.Server.Tests/Live/TestLiveServer.cs @@ -765,6 +765,8 @@ namespace Tgstation.Server.Tests.Live } } } + else + await internalTask; } async Task TestTgsInternal(CancellationToken hardCancellationToken) @@ -845,7 +847,7 @@ namespace Tgstation.Server.Tests.Live } catch (Exception ex) { - System.Console.WriteLine($"[{DateTimeOffset.UtcNow}] TEST ERROR: {ex}"); + Console.WriteLine($"[{DateTimeOffset.UtcNow}] TEST ERROR: {ex}"); serverCts.Cancel(); throw; } From eaf641648a760fa0509cf757747a203763436682 Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 29 May 2023 10:55:06 -0400 Subject: [PATCH 4/8] Fixes streams being passed into IByondClient getting disposed --- build/Version.props | 2 +- src/Tgstation.Server.Client/ApiClient.cs | 24 ++++++++++++------- .../Tgstation.Server.Client.csproj | 2 +- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/build/Version.props b/build/Version.props index 5844d85fea..600c2e4219 100644 --- a/build/Version.props +++ b/build/Version.props @@ -7,7 +7,7 @@ 4.6.0 9.10.2 10.4.1 - 11.4.2 + 11.4.3 6.4.4 5.6.1 1.2.2 diff --git a/src/Tgstation.Server.Client/ApiClient.cs b/src/Tgstation.Server.Client/ApiClient.cs index 649745b609..9913a10184 100644 --- a/src/Tgstation.Server.Client/ApiClient.cs +++ b/src/Tgstation.Server.Client/ApiClient.cs @@ -305,18 +305,26 @@ namespace Tgstation.Server.Client if (content != null) request.Content = content; - var headersToUse = tokenRefresh ? tokenRefreshHeaders! : headers; - headersToUse.SetRequestHeaders(request.Headers, instanceId); + try + { + var headersToUse = tokenRefresh ? tokenRefreshHeaders! : headers; + headersToUse.SetRequestHeaders(request.Headers, instanceId); - if (authless) - request.Headers.Remove(HeaderNames.Authorization); + if (authless) + request.Headers.Remove(HeaderNames.Authorization); - if (fileDownload) - request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue(MediaTypeNames.Application.Octet)); + if (fileDownload) + request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue(MediaTypeNames.Application.Octet)); - await Task.WhenAll(requestLoggers.Select(x => x.LogRequest(request, cancellationToken))).ConfigureAwait(false); + await Task.WhenAll(requestLoggers.Select(x => x.LogRequest(request, cancellationToken))).ConfigureAwait(false); - response = await httpClient.SendAsync(request, cancellationToken).ConfigureAwait(false); + response = await httpClient.SendAsync(request, cancellationToken).ConfigureAwait(false); + } + finally + { + // prevent content param from getting disposed + request.Content = null; + } } try diff --git a/src/Tgstation.Server.Client/Tgstation.Server.Client.csproj b/src/Tgstation.Server.Client/Tgstation.Server.Client.csproj index bd1874f5d5..f171e61f41 100644 --- a/src/Tgstation.Server.Client/Tgstation.Server.Client.csproj +++ b/src/Tgstation.Server.Client/Tgstation.Server.Client.csproj @@ -16,7 +16,7 @@ https://github.com/tgstation/tgstation-server 2018-2023 json web api tgstation-server tgstation ss13 byond client http - Added missing Dispose() call to the StringContents for requests with bodies and added missing ConfigureAwait(false) to async call. + Fixed streams being passed into IByondClient.SetActiveVersion getting disposed. true snupkg ../../build/analyzers.ruleset From ea8f12da0a0d6df334216572e53fd67d7733b6df Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 29 May 2023 10:55:20 -0400 Subject: [PATCH 5/8] Regression test for #1501 --- .../Live/Instance/ByondTest.cs | 29 +++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/tests/Tgstation.Server.Tests/Live/Instance/ByondTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/ByondTest.cs index 84faa305df..8eac19aaf9 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/ByondTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/ByondTest.cs @@ -59,6 +59,12 @@ namespace Tgstation.Server.Tests.Live.Instance async Task TestDeletes(CancellationToken cancellationToken) { + var deleteThisOneBecauseItWasntPartOfTheOriginalTest = await byondClient.DeleteVersion(new ByondVersionDeleteRequest + { + Version = new(TestVersion.Major, TestVersion.Minor, 2) + }, cancellationToken); + await WaitForJob(deleteThisOneBecauseItWasntPartOfTheOriginalTest, 30, false, null, cancellationToken); + var nonExistentUninstallResponseTask = Assert.ThrowsExceptionAsync(() => byondClient.DeleteVersion( new ByondVersionDeleteRequest { @@ -182,14 +188,27 @@ namespace Tgstation.Server.Tests.Live.Instance UploadCustomZip = true }, stableBytesMs, - cancellationToken) - ; + cancellationToken); Assert.IsNotNull(test.InstallJob); - await WaitForJob(test.InstallJob, 60, false, null, cancellationToken); + await WaitForJob(test.InstallJob, 30, false, null, cancellationToken); + + // do it again. #1501 + stableBytesMs.Seek(0, SeekOrigin.Begin); + var test2 = await byondClient.SetActiveVersion( + new ByondVersionRequest + { + Version = TestVersion, + UploadCustomZip = true + }, + stableBytesMs, + cancellationToken); + + Assert.IsNotNull(test2.InstallJob); + await WaitForJob(test2.InstallJob, 30, false, null, cancellationToken); var newSettings = await byondClient.ActiveVersion(cancellationToken); - Assert.AreEqual(new Version(TestVersion.Major, TestVersion.Minor, 1), newSettings.Version); + Assert.AreEqual(new Version(TestVersion.Major, TestVersion.Minor, 2), newSettings.Version); // test a few switches var installResponse = await byondClient.SetActiveVersion(new ByondVersionRequest @@ -199,7 +218,7 @@ namespace Tgstation.Server.Tests.Live.Instance Assert.IsNull(installResponse.InstallJob); await ApiAssert.ThrowsException(() => byondClient.SetActiveVersion(new ByondVersionRequest { - Version = new Version(TestVersion.Major, TestVersion.Minor, 2) + Version = new Version(TestVersion.Major, TestVersion.Minor, 3) }, null, cancellationToken), ErrorCode.ByondNonExistentCustomVersion); installResponse = await byondClient.SetActiveVersion(new ByondVersionRequest From 3e76e386d82078255fe0dbb22ddc46df77832655 Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 29 May 2023 10:58:38 -0400 Subject: [PATCH 6/8] Fixes #1501 --- src/Tgstation.Server.Host/Components/Byond/ByondManager.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Byond/ByondManager.cs b/src/Tgstation.Server.Host/Components/Byond/ByondManager.cs index 49456d01ae..157107228a 100644 --- a/src/Tgstation.Server.Host/Components/Byond/ByondManager.cs +++ b/src/Tgstation.Server.Host/Components/Byond/ByondManager.cs @@ -423,10 +423,10 @@ namespace Tgstation.Server.Host.Components.Byond { if (customVersionStream != null) { - int customInstallationNumber = 1; + var customInstallationNumber = 1; do { - version = new Version(version.Major, version.Minor, customInstallationNumber); + version = new Version(version.Major, version.Minor, customInstallationNumber++); } while (installedVersions.ContainsKey(version)); } From ef13f1eb420f7ad695467a4c90f2c738873f6ada Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 29 May 2023 11:11:26 -0400 Subject: [PATCH 7/8] Bump DMAPI version to label the interop version --- build/Version.props | 2 +- src/DMAPI/tgs.dm | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/build/Version.props b/build/Version.props index 600c2e4219..b1346bca3b 100644 --- a/build/Version.props +++ b/build/Version.props @@ -8,7 +8,7 @@ 9.10.2 10.4.1 11.4.3 - 6.4.4 + 6.4.5 5.6.1 1.2.2 1.2.1 diff --git a/src/DMAPI/tgs.dm b/src/DMAPI/tgs.dm index ab2d565991..d7f7deec74 100644 --- a/src/DMAPI/tgs.dm +++ b/src/DMAPI/tgs.dm @@ -1,6 +1,6 @@ // tgstation-server DMAPI -#define TGS_DMAPI_VERSION "6.4.4" +#define TGS_DMAPI_VERSION "6.4.5" // All functions and datums outside this document are subject to change with any version and should not be relied on. From 2d9a60b393480f2e546575278e043434faff32b8 Mon Sep 17 00:00:00 2001 From: Dominion Date: Mon, 29 May 2023 11:14:45 -0400 Subject: [PATCH 8/8] Version bump to 5.12.3 --- build/Version.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/Version.props b/build/Version.props index b1346bca3b..4aed1e7d1c 100644 --- a/build/Version.props +++ b/build/Version.props @@ -3,7 +3,7 @@ - 5.12.2 + 5.12.3 4.6.0 9.10.2 10.4.1