Possibly fix address reuse issue forever

Child processes may have been ~~inheriting~~ stealing the bind test handle.
tgstation · Oct 16, 2023 · 98e1c19 · 98e1c19
1 parent 7082b7e
commit 98e1c19
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 23 deletions.
diff --git a/src/Tgstation.Server.Host/Components/Session/SessionControllerFactory.cs b/src/Tgstation.Server.Host/Components/Session/SessionControllerFactory.cs
@@ -217,7 +217,6 @@ public async ValueTask<ISessionController> LaunchNew(
 			if (!launchParameters.Port.HasValue)
 				throw new InvalidOperationException("Given port is null!");
 
-			PortBindTest(launchParameters.Port.Value);
 			switch (dmbProvider.CompileJob.MinimumSecurityLevel)
 			{
 				case DreamDaemonSecurity.Ultrasafe:
@@ -255,8 +254,8 @@ public async ValueTask<ISessionController> LaunchNew(
 				var engineType = dmbProvider.EngineVersion.Engine.Value;
 				if (engineType == EngineType.Byond)
 					await CheckPagerIsNotRunning();
-				else if (engineType == EngineType.OpenDream && platformIdentifier.IsWindows)
-					await asyncDelayer.Delay(TimeSpan.FromSeconds(2), cancellationToken); // prevent socket reuse after bind test
+
+				PortBindTest(launchParameters.Port.Value);
 
 				string outputFilePath = null;
 				var preserveLogFile = true;

diff --git a/src/Tgstation.Server.Host/Extensions/SocketExtensions.cs b/src/Tgstation.Server.Host/Extensions/SocketExtensions.cs
@@ -1,6 +1,8 @@
 using System.Net;
 using System.Net.Sockets;
 
+using Tgstation.Server.Host.System;
+
 namespace Tgstation.Server.Host.Extensions
 {
 	/// <summary>
@@ -14,26 +16,27 @@ static class SocketExtensions
 		/// <param name="port">The port number to bind to.</param>
 		/// <param name="includeIPv6">If IPV6 should be tested as well.</param>
 		public static void BindTest(ushort port, bool includeIPv6)
-		{
-			using var socket = new Socket(
-				includeIPv6
-					? AddressFamily.InterNetworkV6
-					: AddressFamily.InterNetwork,
-				SocketType.Stream,
-				ProtocolType.Tcp);
+			=> ProcessExecutor.WithProcessLaunchExclusivity(() =>
+			{
+				using var socket = new Socket(
+					includeIPv6
+						? AddressFamily.InterNetworkV6
+						: AddressFamily.InterNetwork,
+					SocketType.Stream,
+					ProtocolType.Tcp);
 
-			socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.ExclusiveAddressUse, true);
-			socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.ReuseAddress, false);
-			socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.DontLinger, true);
-			if (includeIPv6)
-				socket.DualMode = true;
+				socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.ExclusiveAddressUse, true);
+				socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.ReuseAddress, false);
+				socket.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.DontLinger, true);
+				if (includeIPv6)
+					socket.DualMode = true;
 
-			socket.Bind(
-				new IPEndPoint(
-					includeIPv6
-						? IPAddress.IPv6Any
-						: IPAddress.Any,
-					port));
-		}
+				socket.Bind(
+					new IPEndPoint(
+						includeIPv6
+							? IPAddress.IPv6Any
+							: IPAddress.Any,
+						port));
+			});
 	}
 }
diff --git a/src/Tgstation.Server.Host/System/ProcessExecutor.cs b/src/Tgstation.Server.Host/System/ProcessExecutor.cs
@@ -13,6 +13,11 @@ namespace Tgstation.Server.Host.System
 	/// <inheritdoc />
 	sealed class ProcessExecutor : IProcessExecutor
 	{
+		/// <summary>
+		/// <see cref="ReaderWriterLockSlim"/> for <see cref="WithProcessLaunchExclusivity(Action)"/>.
+		/// </summary>
+		static readonly ReaderWriterLockSlim ExclusiveProcessLaunchLock = new ();
+
 		/// <summary>
 		/// The <see cref="IProcessFeatures"/> for the <see cref="ProcessExecutor"/>.
 		/// </summary>
@@ -33,6 +38,23 @@ sealed class ProcessExecutor : IProcessExecutor
 		/// </summary>
 		readonly ILoggerFactory loggerFactory;
 
+		/// <summary>
+		/// Runs a given <paramref name="action"/> making sure to not launch any processes while its running.
+		/// </summary>
+		/// <param name="action">The <see cref="Action"/> to execute.</param>
+		public static void WithProcessLaunchExclusivity(Action action)
+		{
+			ExclusiveProcessLaunchLock.EnterWriteLock();
+			try
+			{
+				action();
+			}
+			finally
+			{
+				ExclusiveProcessLaunchLock.ExitWriteLock();
+			}
+		}
+
 		/// <summary>
 		/// Initializes a new instance of the <see cref="ProcessExecutor"/> class.
 		/// </summary>
@@ -127,7 +149,15 @@ public IProcess LaunchProcess(
 
 					try
 					{
-						handle.Start();
+						ExclusiveProcessLaunchLock.EnterReadLock();
+						try
+						{
+							handle.Start();
+						}
+						finally
+						{
+							ExclusiveProcessLaunchLock.ExitReadLock();
+						}
 
 						processStartTcs?.SetResult();
 					}