Merge pull request #1789 from tgstation/TheLastSpuriousCIError

[MUST PASS CI 10 CONSECUTIVE TIMES BEFORE MERGING] Workaround for world.Export potentially hanging forever

build/Version.props

@@ -9,7 +9,7 @@
     <TgsCommonLibraryVersion>7.0.0</TgsCommonLibraryVersion>
     <TgsApiLibraryVersion>13.2.0</TgsApiLibraryVersion>
     <TgsClientVersion>15.2.0</TgsClientVersion>
-    <TgsDmapiVersion>7.1.0</TgsDmapiVersion>
+    <TgsDmapiVersion>7.1.1</TgsDmapiVersion>
     <TgsInteropVersion>5.9.0</TgsInteropVersion>
     <TgsHostWatchdogVersion>1.4.1</TgsHostWatchdogVersion>
     <TgsContainerScriptVersion>1.2.1</TgsContainerScriptVersion>

src/DMAPI/tgs.dm

@@ -1,6 +1,6 @@
 // tgstation-server DMAPI
 
-#define TGS_DMAPI_VERSION "7.1.0"
+#define TGS_DMAPI_VERSION "7.1.1"
 
 // All functions and datums outside this document are subject to change with any version and should not be relied on.
 
@@ -496,7 +496,7 @@
 /// Returns a list of connected [/datum/tgs_chat_channel]s if TGS is present, null otherwise. This function may sleep if the call to [/world/proc/TgsNew] is sleeping!
 /world/proc/TgsChatChannelInfo()
 	return
-	
+
 /**
  * Trigger an event in TGS. Requires TGS version >= 6.3.0. Returns [TRUE] if the event was triggered successfully, [FALSE] otherwise. This function may sleep!
  *

src/DMAPI/tgs/v5/api.dm

@@ -48,6 +48,10 @@
 
 	var/datum/tgs_version/api_version = ApiVersion()
 	version = null // we want this to be the TGS version, not the interop version
+
+	// sleep once to prevent an issue where world.Export on the first tick can hang indefinitely
+	sleep(world.tick_lag)
+
 	var/list/bridge_response = Bridge(DMAPI5_BRIDGE_COMMAND_STARTUP, list(DMAPI5_BRIDGE_PARAMETER_MINIMUM_SECURITY_LEVEL = minimum_required_security_level, DMAPI5_BRIDGE_PARAMETER_VERSION = api_version.raw_parameter, DMAPI5_PARAMETER_CUSTOM_COMMANDS = ListCustomCommands(), DMAPI5_PARAMETER_TOPIC_PORT = GetTopicPort()))
 	if(!istype(bridge_response))
 		TGS_ERROR_LOG("Failed initial bridge request!")

src/DMAPI/tgs/v5/topic.dm

@@ -177,7 +177,8 @@
 			reattach_response[DMAPI5_PARAMETER_CUSTOM_COMMANDS] = ListCustomCommands()
 			reattach_response[DMAPI5_PARAMETER_TOPIC_PORT] = GetTopicPort()
 
-			pending_events.Cut()
+			for(var/eventId in pending_events)
+				pending_events[eventId] = TRUE
 
 			return reattach_response
 

src/Tgstation.Server.Host/System/WindowsProcessFeatures.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
 using System.Linq;
@@ -68,28 +69,40 @@ public void SuspendProcess(global::System.Diagnostics.Process process)
 		{
 			ArgumentNullException.ThrowIfNull(process);
 
-			process.Refresh();
-			foreach (ProcessThread thread in process.Threads)
+			var suspendedThreadIds = new HashSet<uint>();
+			bool suspendedNewThreads;
+			do
 			{
-				var threadId = (uint)thread.Id;
-				logger.LogTrace("Suspending thread {threadId}...", threadId);
-				var pOpenThread = NativeMethods.OpenThread(NativeMethods.ThreadAccess.SuspendResume, false, threadId);
-				if (pOpenThread == IntPtr.Zero)
-				{
-					logger.LogDebug(new Win32Exception(), "Failed to open thread {threadId}!", threadId);
-					continue;
-				}
-
-				try
-				{
-					if (NativeMethods.SuspendThread(pOpenThread) == UInt32.MaxValue)
-						throw new Win32Exception();
-				}
-				finally
+				suspendedNewThreads = false;
+				process.Refresh();
+				foreach (ProcessThread thread in process.Threads)
 				{
-					NativeMethods.CloseHandle(pOpenThread);
+					var threadId = (uint)thread.Id;
+
+					if (!suspendedThreadIds.Add(threadId))
+						continue;
+
+					suspendedNewThreads = true;
+					logger.LogTrace("Suspending thread {threadId}...", threadId);
+					var pOpenThread = NativeMethods.OpenThread(NativeMethods.ThreadAccess.SuspendResume, false, threadId);
+					if (pOpenThread == IntPtr.Zero)
+					{
+						logger.LogDebug(new Win32Exception(), "Failed to open thread {threadId}!", threadId);
+						continue;
+					}
+
+					try
+					{
+						if (NativeMethods.SuspendThread(pOpenThread) == UInt32.MaxValue)
+							throw new Win32Exception();
+					}
+					finally
+					{
+						NativeMethods.CloseHandle(pOpenThread);
+					}
 				}
 			}
+			while (suspendedNewThreads);
 		}
 
 		/// <inheritdoc />

tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs

@@ -526,7 +526,7 @@ async Task DumpTests(bool mini, CancellationToken cancellationToken)
 			}, cancellationToken);
 			Assert.AreEqual(mini, updated.Minidumps);
 			var dumpJob = await instanceClient.DreamDaemon.CreateDump(cancellationToken);
-			await WaitForJob(dumpJob, 30, false, null, cancellationToken);
+			await WaitForJob(dumpJob, 60, false, null, cancellationToken);
 
 			var dumpFiles = Directory.GetFiles(Path.Combine(
 				instanceClient.Metadata.Path, "Diagnostics", "ProcessDumps"), testVersion.Engine == EngineType.OpenDream ? "*.net.dmp" : "*.dmp");
@@ -729,28 +729,41 @@ void TestLinuxIsntBeingFuckingCheekyAboutFilePaths(DreamDaemonResponse currentSt
 			var foundLivePath = false;
 			var allPaths = new List<string>();
 
-			Assert.IsFalse(proc.HasExited);
-			foreach (var fd in Directory.GetFiles($"/proc/{pid}/fd"))
+			var features = new PosixProcessFeatures(
+				new Lazy<IProcessExecutor>(Mock.Of<IProcessExecutor>()),
+				Mock.Of<IIOManager>(),
+				Mock.Of<ILogger<PosixProcessFeatures>>());
+
+			features.SuspendProcess(proc);
+			try
 			{
-				var sb = new StringBuilder(UInt16.MaxValue);
-				if (Syscall.readlink(fd, sb) == -1)
-					throw new UnixIOException(Stdlib.GetLastError());
+				Assert.IsFalse(proc.HasExited);
+				foreach (var fd in Directory.GetFiles($"/proc/{pid}/fd"))
+				{
+					var sb = new StringBuilder(UInt16.MaxValue);
+					if (Syscall.readlink(fd, sb) == -1)
+						throw new UnixIOException(Stdlib.GetLastError());
 
-				var path = sb.ToString();
+					var path = sb.ToString();
 
-				allPaths.Add($"Path: {path}");
-				if (path.Contains($"Game/{previousStatus.DirectoryName}"))
-					failingLinks.Add($"Found fd {fd} resolving to previous absolute path game dir path: {path}");
+					allPaths.Add($"Path: {path}");
+					if (path.Contains($"Game/{previousStatus.DirectoryName}"))
+						failingLinks.Add($"Found fd {fd} resolving to previous absolute path game dir path: {path}");
 
-				if (path.Contains($"Game/{currentStatus.ActiveCompileJob.DirectoryName}"))
-					failingLinks.Add($"Found fd {fd} resolving to current absolute path game dir path: {path}");
+					if (path.Contains($"Game/{currentStatus.ActiveCompileJob.DirectoryName}"))
+						failingLinks.Add($"Found fd {fd} resolving to current absolute path game dir path: {path}");
 
-				if (path.Contains($"Game/Live"))
-					foundLivePath = true;
-			}
+					if (path.Contains($"Game/Live"))
+						foundLivePath = true;
+				}
 
-			if (!foundLivePath)
-				failingLinks.Add($"Failed to find a path containing the 'Live' directory!");
+				if (!foundLivePath)
+					failingLinks.Add($"Failed to find a path containing the 'Live' directory!");
+			}
+			finally
+			{
+				features.ResumeProcess(proc);
+			}
 
 			Assert.IsTrue(failingLinks.Count == 0, String.Join(Environment.NewLine, failingLinks.Concat(allPaths)));
 		}