From fe22f1bd4d3a2fd3fb239ff6efc503865031888c Mon Sep 17 00:00:00 2001 From: vibesoftwarecoder Date: Fri, 11 Sep 2026 08:10:13 -0500 Subject: [PATCH] fix(monitoring): report a standalone Apollo as streaming when it actually is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses #23, and found a live bug on the way. ## The signal was wrong HostApolloInfo.Streaming came from Apollo's serverinfo alone: state not ending in FREE, or currentgame != 0. Those describe a launched APPLICATION, not a client session. Someone streaming the plain desktop -- which is what a console Apollo is usually for -- leaves currentgame at 0 and state at SUNSHINE_SERVER_FREE for the entire session. Measured 2026-09-11: the standalone Apollo encoding steadily at ~15.8% for 26 minutes, with an encoder created in its log and never torn down, still answered state = SUNSHINE_SERVER_FREE, currentgame = 0. So /api/host showed the dashboard "not streaming" for the whole time the host WAS streaming. Streaming is now per-process GPU video encode OR the serverinfo signal, read through the same WMI GPU engine counters GpuMonitor already uses, filtered to the Apollo PID and the videoencode engine. Per process, never the GPU total -- RustDesk encodes too, and a machine-wide reading answers a different question. Verified in both directions on the reference host: False while idle with serverinfo FREE, and True during a live stream where serverinfo still insisted it was idle. ## The warning #23 asked for #23's decomposition established the stall is not fixable here: stopping a seat's Apollo causes no reaction at all; ending the seat's RDP SESSION is the desktop topology change any Apollo on the host reacts to, and a seat is an RDP session. What was wrong was that it happened invisibly -- the operator interrupted someone and had no way to know. SeatManager now logs before teardown when the standalone Apollo is streaming, naming the ~690 ms stall and that it self-recovers. It runs before the lifecycle gate is taken, so it adds no latency under the lock, and it can never block or fail a teardown -- a seat that would not tear down because a status query timed out is a far worse bug than the one it reports. ⛔ Built on the OLD signal this warning would have been silently dead for desktop streaming, which is the common case. That is why the signal was checked before being relied on rather than after. CLAUDE.md's non-destructive coexistence claim now states the exception, and that the cause is session churn rather than a second Apollo. ⚠️ Not yet verified: that the warning fires during a real teardown, and stays quiet when nothing is streaming. Both need seat churn and the host was streaming. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SQvL62WkT8xDWXqyjFCGDw --- CLAUDE.md | 17 ++++++ .../Monitoring/HostApolloMonitor.cs | 59 ++++++++++++++++++- src/MultiSeat.Service/Sessions/SeatManager.cs | 50 ++++++++++++++++ .../Sessions/SeatTeardownGuardTests.cs | 4 ++ 4 files changed, 129 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 650956d..c9470dd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -342,6 +342,23 @@ MultiSeat is self-contained and **non-destructive**: it works out of the box whe 2. **Own port range.** Default `PortBase = 48100`, above a stock Apollo's block — no runtime port conflict. 3. **Never kills a non-MultiSeat Apollo.** On startup `MultiSeatWorker.KillOrphanedApolloProcesses` reaps **only** Apollo processes MultiSeat launched, identified via WMI (`GetManagedApolloPids`) by executable path (under the ApolloVibe dir) or a MultiSeat per-seat config path on the command line. It no longer stops/disables `ApolloService`, and `install-service.ps1` leaves that service alone. (WMI failure → empty set → cleanup is skipped rather than risk killing an unrelated Apollo.) +⚠️ **"Non-destructive" has one exception, and it is not fixable here: tearing a seat down stalls a +standalone Apollo's stream for about 690 ms** while it rebuilds its encoder. Self-recovering, and +it does not compound with seat count — seat-to-seat interference was measured and does not exist. + +⛔ **The cause is NOT a second Apollo starting or stopping**, which is the intuitive guess and was +this issue's original premise. Decomposed step by step (#23): stopping the seat's Apollo produced +*no reaction at all*; killing mstsc and logging the session off each produced the rebuild. **A seat +IS an RDP session, and that session's display appearing and disappearing is a desktop topology +change** — which invalidates any DXGI duplication on the host, including one MultiSeat has nothing +to do with. It connects directly to #15: because seats stream the RDP surface rather than a virtual +display, seat churn *is* display churn. + +There is no VDD detach to defer or reorder, so the stall stays. What `SeatManager` does now is +**say so**: `WarnIfStandaloneApolloStreamingAsync` logs a warning before teardown when the +standalone Apollo reports it is streaming. ⚠️ It never blocks or fails a teardown — a seat that +would not tear down because a status query timed out is a far worse bug than the one it reports. + ## Shared game library & emulator netplay Because each seat is its own Windows account, games/ROMs would otherwise be siloed per account and seats couldn't easily netplay. Two provisioning helpers address this (config in `MultiSeatOptions` / `appsettings.json`): diff --git a/src/MultiSeat.Service/Monitoring/HostApolloMonitor.cs b/src/MultiSeat.Service/Monitoring/HostApolloMonitor.cs index c1ca1ad..7c6e4e2 100644 --- a/src/MultiSeat.Service/Monitoring/HostApolloMonitor.cs +++ b/src/MultiSeat.Service/Monitoring/HostApolloMonitor.cs @@ -195,7 +195,64 @@ private async Task QueryServerInfoAsync(HostApolloInfo info, int port, Cancellat info.Reachable = true; info.HostName = server.HostName; info.AppVersion = server.AppVersion; - info.Streaming = server.Streaming; + + // ⛔ serverinfo ALONE gets this wrong, and wrong in the common direction. + // + // Its `state` and `currentgame` describe a launched APPLICATION, not a client session. + // Someone streaming the plain desktop — which is what a console Apollo is usually for — + // leaves currentgame at 0 and state at SUNSHINE_SERVER_FREE for the whole session. + // + // Measured 2026-09-11: the standalone Apollo encoding steadily at ~15.8% for 26 minutes, + // with an encoder created in its log and never torn down, still answered + // `state = SUNSHINE_SERVER_FREE, currentgame = 0`. Anything built on this flag alone is + // silently dead for desktop streaming. + // + // Per-process GPU video encode is the signal that actually tracks a client, and is the + // rule this project already follows operationally. serverinfo is kept as an OR because it + // still catches a launched game whose client is momentarily not encoding. + info.Streaming = IsProcessVideoEncoding(info.ProcessId) || server.Streaming; + } + + /// + /// Is this specific process feeding the GPU's video encoder right now? + /// + /// Uses the same WMI GPU engine counters reads, filtered to one PID + /// and to the video-encode engine. The instance name looks like + /// pid_10988_luid_0x00000000_0x0001132D_phys_0_eng_6_engtype_videoencode. + /// + /// ⚠️ Per PROCESS, never the GPU total. RustDesk and any other remote-desktop tool encode too, + /// so a machine-wide reading says "something is encoding", which is not the question. + /// + private bool IsProcessVideoEncoding(int pid) + { + if (pid <= 0) return false; + + try + { + using var searcher = new System.Management.ManagementObjectSearcher( + "SELECT Name, UtilizationPercentage FROM " + + "Win32_PerfFormattedData_GPUPerformanceCounters_GPUEngine"); + + var marker = $"pid_{pid}_"; + foreach (var obj in searcher.Get()) + { + var name = obj["Name"]?.ToString() ?? string.Empty; + if (!name.StartsWith(marker, StringComparison.OrdinalIgnoreCase)) continue; + if (!name.Contains("engtype_videoencode", StringComparison.OrdinalIgnoreCase)) continue; + + // A live stream sits well above this; an idle process reads 0. The threshold only + // has to separate "encoding" from "not", not measure anything. + if (Convert.ToDouble(obj["UtilizationPercentage"] ?? 0) > 1.0) return true; + } + } + catch (Exception ex) + { + // Counters unavailable — report not-encoding rather than guessing. Callers treat this + // as advisory, so a false negative costs a warning, not correctness. + _logger.LogDebug(ex, "Could not read GPU encode counters for PID {Pid}", pid); + } + + return false; } /// ApolloService state, or null when the service is not installed. diff --git a/src/MultiSeat.Service/Sessions/SeatManager.cs b/src/MultiSeat.Service/Sessions/SeatManager.cs index f638b3b..2c74b13 100644 --- a/src/MultiSeat.Service/Sessions/SeatManager.cs +++ b/src/MultiSeat.Service/Sessions/SeatManager.cs @@ -50,6 +50,12 @@ public sealed class SeatManager private readonly HidHideConfigurator _hidHide; private readonly OnConnectAppLauncher _onConnectApps; private readonly Monitoring.ApolloServerQuery _serverQuery; + + /// + /// Used only to warn when a seat teardown is about to disturb a standalone Apollo's stream. + /// See and issue #23. + /// + private readonly Monitoring.HostApolloMonitor _hostApollo; private readonly IEnumerable _emulatorSeeders; private readonly SeatLifecycleGate _lifecycleGate; @@ -71,6 +77,7 @@ public SeatManager( HidHideConfigurator hidHide, OnConnectAppLauncher onConnectApps, Monitoring.ApolloServerQuery serverQuery, + Monitoring.HostApolloMonitor hostApollo, IEnumerable emulatorSeeders, SeatLifecycleGate lifecycleGate) { @@ -91,6 +98,7 @@ public SeatManager( _hidHide = hidHide; _onConnectApps = onConnectApps; _serverQuery = serverQuery; + _hostApollo = hostApollo; _emulatorSeeders = emulatorSeeders; _lifecycleGate = lifecycleGate; } @@ -583,6 +591,11 @@ internal async Task TeardownSeatAsync(Guid seatId, TimeSpan gateTimeout, Cancell if (GetSeat(seatId) is null) return; + // Say so BEFORE the stall happens, not after, and before the gate is taken — this is + // advisory, and holding the lifecycle gate across a network query would slow every + // teardown to buy nothing. + await WarnIfStandaloneApolloStreamingAsync(seatId, ct); + // ⛔ Acquire the gate BEFORE removing the seat from _seats, not after. // // The reverse order looks harmless and is not: AcquireAsync throws TimeoutException after @@ -611,6 +624,43 @@ internal async Task TeardownSeatAsync(Guid seatId, TimeSpan gateTimeout, Cancell _logger.LogInformation("Seat {Id}: torn down", seat.Id); } + /// + /// Warn when tearing this seat down is about to interrupt someone else's stream. + /// + /// A seat IS an RDP session, and that session's display appearing and disappearing is a + /// desktop topology change. Any Apollo on the host sees it and rebuilds its capture pipeline — + /// measured at roughly 690 ms, self-recovering, on the standalone console Apollo (#23). + /// + /// ⛔ This does NOT prevent the stall, and is not trying to. The trigger is the RDP session + /// ending, which is exactly what tearing a seat down means; it was decomposed and the seat's + /// own Apollo stopping turned out not to be the cause at all. What was wrong was that it + /// happened INVISIBLY — the operator interrupted someone and had no way to know. + /// + /// ⚠️ Never blocks or fails the teardown. A seat that will not tear down because a status + /// query timed out would be a far worse bug than the one this reports. + /// + private async Task WarnIfStandaloneApolloStreamingAsync(Guid seatId, CancellationToken ct) + { + try + { + var host = await _hostApollo.CollectAsync(ct); + if (host.Detected && host.Streaming) + { + _logger.LogWarning( + "Seat {Id}: tearing down while the standalone Apollo (PID {Pid}, {Name}) is " + + "streaming. Ending this seat's RDP session changes the desktop topology, so " + + "that stream will stall for about a second while its encoder rebuilds. It " + + "recovers on its own. See issue #23.", + seatId, host.ProcessId, host.HostName ?? "unnamed"); + } + } + catch (Exception ex) + { + // Advisory only. Losing the warning is acceptable; losing the teardown is not. + _logger.LogDebug(ex, "Seat {Id}: could not check the standalone Apollo before teardown", seatId); + } + } + /// /// Teardown all seats — called on service shutdown. /// diff --git a/src/MultiSeat.Tests/Sessions/SeatTeardownGuardTests.cs b/src/MultiSeat.Tests/Sessions/SeatTeardownGuardTests.cs index 76f9263..18f0ff6 100644 --- a/src/MultiSeat.Tests/Sessions/SeatTeardownGuardTests.cs +++ b/src/MultiSeat.Tests/Sessions/SeatTeardownGuardTests.cs @@ -114,6 +114,10 @@ public void ResolveLiveSeat_AcceptsEveryNonTeardownStatus(SeatStatus status) apolloManager: null!, configBuilder: null!, portAllocator: null!, firewall: null!, audioRouter: null!, controllerManager: null!, inputRouter: null!, inputHookManager: null!, hidHide: null!, onConnectApps: null!, serverQuery: null!, + // Null on purpose. Teardown consults this only to warn about disturbing a standalone + // Apollo's stream, and that check must never fail a teardown — so the timeout path below + // exercises exactly that: the warning blows up internally and teardown proceeds regardless. + hostApollo: null!, emulatorSeeders: Array.Empty(), lifecycleGate: gate);