Fix instance stop timeout not killing the process

Caused by 72ddaf91.

Agent/Phantom.Agent.Minecraft/Server/ServerStatusProtocol.cs

@@ -3,11 +3,28 @@ using System.Buffers.Binary;
 using System.Net;
 using System.Net.Sockets;
 using System.Text;
+using Phantom.Utils.Logging;
+using Serilog;
 
 namespace Phantom.Agent.Minecraft.Server;
 
-public static class ServerStatusProtocol {
+public sealed class ServerStatusProtocol {
-	public static async Task<int> GetOnlinePlayerCount(ushort serverPort, CancellationToken cancellationToken) {
+	private readonly ILogger logger;
+
+	public ServerStatusProtocol(string loggerName) {
+		this.logger = PhantomLogger.Create<ServerStatusProtocol>(loggerName);
+	}
+
+	public async Task<int?> GetOnlinePlayerCount(int serverPort, CancellationToken cancellationToken) {
+		try {
+			return await GetOnlinePlayerCountOrThrow(serverPort, cancellationToken);
+		} catch (Exception e) {
+			logger.Error(e, "Caught exception checking online player count.");
+			return null;
+		}
+	}
+
+	private async Task<int?> GetOnlinePlayerCountOrThrow(int serverPort, CancellationToken cancellationToken) {
 		using var tcpClient = new TcpClient();
 		await tcpClient.ConnectAsync(IPAddress.Loopback, serverPort, cancellationToken);
 		var tcpStream = tcpClient.GetStream();
@@ -16,22 +33,24 @@ public static class ServerStatusProtocol {
 		tcpStream.WriteByte(0xFE);
 		await tcpStream.FlushAsync(cancellationToken);
 
-		short messageLength = await ReadStreamHeader(tcpStream, cancellationToken);
+		short? messageLength = await ReadStreamHeader(tcpStream, cancellationToken);
-		return await ReadOnlinePlayerCount(tcpStream, messageLength * 2, cancellationToken);
+		return messageLength == null ? null : await ReadOnlinePlayerCount(tcpStream, messageLength.Value * 2, cancellationToken);
 	}
 
-	private static async Task<short> ReadStreamHeader(NetworkStream tcpStream, CancellationToken cancellationToken) {
+	private async Task<short?> ReadStreamHeader(NetworkStream tcpStream, CancellationToken cancellationToken) {
 		var headerBuffer = ArrayPool<byte>.Shared.Rent(3);
 		try {
 			await tcpStream.ReadExactlyAsync(headerBuffer, 0, 3, cancellationToken);
 
 			if (headerBuffer[0] != 0xFF) {
-				throw new ProtocolException("Unexpected first byte in response from server: " + headerBuffer[0]);
+				logger.Error("Unexpected first byte in response from server: {FirstByte}.", headerBuffer[0]);
+				return null;
 			}
 
 			short messageLength = BinaryPrimitives.ReadInt16BigEndian(headerBuffer.AsSpan(1));
 			if (messageLength <= 0) {
-				throw new ProtocolException("Unexpected message length in response from server: " + messageLength);
+				logger.Error("Unexpected message length in response from server: {MessageLength}.", messageLength);
+				return null;
 			}
 			
 			return messageLength;
@@ -40,7 +59,7 @@ public static class ServerStatusProtocol {
 		}
 	}
 
-	private static async Task<int> ReadOnlinePlayerCount(NetworkStream tcpStream, int messageLength, CancellationToken cancellationToken) {
+	private async Task<int?> ReadOnlinePlayerCount(NetworkStream tcpStream, int messageLength, CancellationToken cancellationToken) {
 		var messageBuffer = ArrayPool<byte>.Shared.Rent(messageLength);
 		try {
 			await tcpStream.ReadExactlyAsync(messageBuffer, 0, messageLength, cancellationToken);
@@ -55,21 +74,20 @@ public static class ServerStatusProtocol {
 			int separator2 = Array.LastIndexOf(messageBuffer, SeparatorSecondByte);
 			int separator1 = separator2 == -1 ? -1 : Array.LastIndexOf(messageBuffer, SeparatorSecondByte, separator2 - 1);
 			if (!IsValidSeparator(messageBuffer, separator1) || !IsValidSeparator(messageBuffer, separator2)) {
-				throw new ProtocolException("Could not find message separators in response from server.");
+				logger.Error("Could not find message separators in response from server.");
+				return null;
 			}
 
 			string onlinePlayerCountStr = Encoding.BigEndianUnicode.GetString(messageBuffer.AsSpan((separator1 + 1)..(separator2 - 1)));
 			if (!int.TryParse(onlinePlayerCountStr, out int onlinePlayerCount)) {
-				throw new ProtocolException("Could not parse online player count in response from server: " + onlinePlayerCountStr);
+				logger.Error("Could not parse online player count in response from server: {OnlinePlayerCount}.", onlinePlayerCountStr);
+				return null;
 			}
 			
+			logger.Debug("Detected {OnlinePlayerCount} online player(s).", onlinePlayerCount);
 			return onlinePlayerCount;
 		} finally {
 			ArrayPool<byte>.Shared.Return(messageBuffer);
 		}
 	}
-	
-	public sealed class ProtocolException : Exception {
-		internal ProtocolException(string message) : base(message) {}
-	}
 }

Agent/Phantom.Agent.Services/Instances/State/InstancePlayerCountTracker.cs

@@ -10,6 +10,8 @@ sealed class InstancePlayerCountTracker : CancellableBackgroundTask {
 	private readonly InstanceProcess process;
 	private readonly ushort serverPort;
 
+	private readonly ServerStatusProtocol serverStatusProtocol;
+
 	private readonly TaskCompletionSource firstDetection = AsyncTasks.CreateCompletionSource();
 	private readonly ManualResetEventSlim serverOutputEvent = new ();
 
@@ -43,6 +45,7 @@ sealed class InstancePlayerCountTracker : CancellableBackgroundTask {
 	public InstancePlayerCountTracker(InstanceContext context, InstanceProcess process, ushort serverPort) : base(PhantomLogger.Create<InstancePlayerCountTracker>(context.ShortName)) {
 		this.process = process;
 		this.serverPort = serverPort;
+		this.serverStatusProtocol = new ServerStatusProtocol(context.ShortName);
 		Start();
 	}
 
@@ -56,7 +59,7 @@ sealed class InstancePlayerCountTracker : CancellableBackgroundTask {
 		while (!CancellationToken.IsCancellationRequested) {
 			serverOutputEvent.Reset();
 
-			OnlinePlayerCount = await TryGetOnlinePlayerCount();
+			OnlinePlayerCount = await serverStatusProtocol.GetOnlinePlayerCount(serverPort, CancellationToken);
 			
 			if (!firstDetection.Task.IsCompleted) {
 				firstDetection.SetResult();
@@ -64,21 +67,7 @@ sealed class InstancePlayerCountTracker : CancellableBackgroundTask {
 
 			await Task.Delay(TimeSpan.FromSeconds(10), CancellationToken);
 			await serverOutputEvent.WaitHandle.WaitOneAsync(CancellationToken);
-			await Task.Delay(TimeSpan.FromSeconds(1), CancellationToken);
+			await Task.Delay(TimeSpan.FromSeconds(2), CancellationToken);
-		}
-	}
-
-	private async Task<int?> TryGetOnlinePlayerCount() {
-		try {
-			int newOnlinePlayerCount = await ServerStatusProtocol.GetOnlinePlayerCount(serverPort, CancellationToken);
-			Logger.Debug("Detected {OnlinePlayerCount} online player(s).", newOnlinePlayerCount);
-			return newOnlinePlayerCount;
-		} catch (ServerStatusProtocol.ProtocolException e) {
-			Logger.Error(e.Message);
-			return null;
-		} catch (Exception e) {
-			Logger.Error(e, "Caught exception while checking online player count.");
-			return null;
 		}
 	}