diff --git a/DS4Windows/DS4Control/ControlService.cs b/DS4Windows/DS4Control/ControlService.cs index bc58ac2..5281e74 100644 --- a/DS4Windows/DS4Control/ControlService.cs +++ b/DS4Windows/DS4Control/ControlService.cs @@ -1740,15 +1740,21 @@ public bool Start(bool showlog = true) AssignInitialDevices(); StartupDiag("AssignInitialDevices end"); - // A force-closed prior development build can leave its - // USB/IP output imported. Remove those ports before HID - // discovery or DS4Windows will ingest its own VIIPER DS4, - // create a second output/UAC endpoint, and recurse. - ViiperUsbipPortManager.DetachStaleLocalViiperPorts(); - // Let usbccgp/HID finish publishing removal before the - // first input snapshot; otherwise a detached interface can - // remain enumerable for one final discovery pass. - Thread.Sleep(250); + // This used to detach "stale" local VIIPER imports before + // HID discovery so a force-closed prior session's output + // could not be ingested as an input and recursed on. It + // detaches nothing now: identified only by controller + // VID/PID and a localhost URL, another application's live + // virtual pad is indistinguishable from a leftover, and on + // 2026-07-31 the sweep disconnected one mid-game. The pass + // still runs for its log line — any unmanaged local import + // is named, with a pointer to the Settings backend-process + // card, which can attribute leftovers and clear them with + // consent. The self-ingestion case that motivated the + // detach is accepted as a residual risk until inputs can + // recognise this app's own virtual pads; the startup + // warning above fires for exactly that state. + ViiperUsbipPortManager.ObserveLocalImports(); StartupDiag("DS4Devices.findControllers dispatch begin"); eventDispatcher.Invoke(() => diff --git a/DS4Windows/DS4Control/Viiper/ViiperBackendDebugger.cs b/DS4Windows/DS4Control/Viiper/ViiperBackendDebugger.cs index da8b1a2..72b1934 100644 --- a/DS4Windows/DS4Control/Viiper/ViiperBackendDebugger.cs +++ b/DS4Windows/DS4Control/Viiper/ViiperBackendDebugger.cs @@ -226,15 +226,15 @@ private void RunDeviceProbe(ViiperVirtualDeviceType type, CancellationToken canc int feedbackLength = ViiperStatePacketBuilder.GetFeedbackLength(type); Log($"Device={type} viiperName={viiperDeviceName} packetLength={packetLength} feedbackLength={feedbackLength}"); - // The stale-import sweep used to live inside CreateDeviceAndOpenStream; - // it now gates the output ladder in ViiperOutDevice instead, so this - // diagnostic runs its own. Reported rather than enforced: the point of - // the debugger is to say what it found. - ViiperStalePortSweep sweep = - ViiperUsbipPortManager.DetachStaleLocalViiperPorts(); - Log(sweep.Cleared - ? "Stale local VIIPER imports: none present" - : $"Stale local VIIPER imports UNPROVEN: {sweep.Reason}"); + // The observation pass that gates the output ladder in + // ViiperOutDevice; run here too so the diagnostic records the + // same view. It only reads — unmanaged imports are logged by + // the pass itself and never touched. + ViiperImportObservation observation = + ViiperUsbipPortManager.ObserveLocalImports(); + Log(observation.Observed + ? "Imported usbip ports: readable" + : $"Imported usbip ports UNREADABLE: {observation.Reason}"); using ViiperDeviceStream stream = client.CreateDeviceAndOpenStream(type); Log($"Device={type} create/open stream OK"); diff --git a/DS4Windows/DS4Control/Viiper/ViiperOutDevice.cs b/DS4Windows/DS4Control/Viiper/ViiperOutDevice.cs index a8c7830..73104ad 100644 --- a/DS4Windows/DS4Control/Viiper/ViiperOutDevice.cs +++ b/DS4Windows/DS4Control/Viiper/ViiperOutDevice.cs @@ -12,6 +12,7 @@ it under the terms of the GNU General Public License as published by using System.Buffers.Binary; using System.Collections.Generic; using System.Diagnostics; +using System.Globalization; using System.IO; using System.Net.Sockets; using System.Text; @@ -783,24 +784,25 @@ private bool ApplyVirtualDeviceGate() private ViiperDeviceStream CreateDeviceStream() { - // Unproven removal blocks reuse: creating a device while a previous import - // may still be attached is what this refuses. "The port list could not be - // read" is not evidence that nothing is there, and the sweep tells those - // apart. IOException matches how the audio gate above refuses, so the - // callers that already handle a refused creation handle this one too. + // Unproven observation blocks creation: "the port list could not be + // read" is not evidence that nothing is there (3.3's rule, kept). + // What no longer blocks — or gets detached — is an import this + // session does not manage: it cannot be attributed from here, our + // own removals are transactional in-session, and the new device + // gets a fresh bus either way. IOException matches how the audio + // gate above refuses, so the callers that already handle a refused + // creation handle this one too. // // Done here, at the ladder entry, rather than inside - // CreateDeviceAndOpenStream: every persona rung calls that, and sweeping - // per rung repeated a loop that can run for seconds. - ViiperStalePortSweep sweep = - ViiperUsbipPortManager.DetachStaleLocalViiperPorts(); - if (!sweep.Cleared) + // CreateDeviceAndOpenStream: every persona rung calls that. + ViiperImportObservation observation = + ViiperUsbipPortManager.ObserveLocalImports(); + if (!observation.Observed) { throw new IOException( - "Refusing to create a virtual controller: " + sweep.Reason + + "Refusing to create a virtual controller: " + observation.Reason + ". A previous virtual controller may still be attached, and " + - "creating another before that is settled risks two devices " + - "claiming the same identity."); + "with the port list unreadable there is no way to tell."); } activeStreamUsesFramedProtocol = false; @@ -4834,7 +4836,15 @@ public ViiperDeviceStream CreateDeviceAndOpenStream(string deviceName, }, JsonOptions); device = SendRequest($"bus/{bus.BusId}/add", payload); - usbipPort = ViiperUsbipPortManager.FindLocalViiperPort(bus.BusId, device.DevId); + // A backend recent enough to report where it attached the + // device is believed over any scan of the port table: the scan + // matches by bus id, and bus ids can collide across two local + // servers. Older backends omit the field (0), so the scan + // remains the fallback — with its ambiguity now failing to -1 + // rather than guessing. + usbipPort = device.UsbipPort > 0 + ? device.UsbipPort + : ViiperUsbipPortManager.FindLocalViiperPort(bus.BusId, device.DevId); ViiperUsbipPortManager.RegisterActivePort(usbipPort); ViiperUsbipPortManager.DetachDuplicateLocalViiperPorts(bus.BusId, device.DevId, usbipPort); return OpenStream(bus.BusId, device.DevId, usbipPort); @@ -5084,6 +5094,15 @@ private sealed class ViiperDeviceResponse { [JsonPropertyName("devId")] public string DevId { get; set; } + + /// + /// The usbip port the backend auto-attached this device on. + /// Serialized with omitempty upstream, so a backend that + /// predates the field (added 2026-07-30) leaves it 0 here — usbip + /// ports count from 1, so 0 reads as "not reported". + /// + [JsonPropertyName("usbipPort")] + public int UsbipPort { get; set; } } private sealed class ViiperDeviceCreateRequest @@ -5134,56 +5153,80 @@ private sealed class ViiperApiError } /// - /// Outcome of a stale-import sweep. means the machine was - /// observed free of stale local VIIPER imports — not merely that nothing went - /// wrong. An unproven sweep carries the reason so a refusal can say it. + /// Outcome of looking at the machine's imported usbip ports. + /// means the port list was actually read — not that + /// it was empty, and not that anything was done about its contents. An + /// unobserved result carries the reason so a refusal can say it. + /// + /// This used to be the verdict of a sweep that also detached + /// what it took for leftovers. It no longer detaches anything: on + /// 2026-07-31 the sweep identified "ours" by controller VID/PID plus a + /// localhost server URL, which is also exactly what another application's + /// live virtual pad looks like, and it disconnected one mid-game. Imports + /// this session did not create cannot be attributed from here — a dead + /// session's leftover and another program's controller are + /// indistinguishable — so they are reported, never touched. The recovery + /// path for real leftovers is the Settings backend-process card, which + /// shows the holdings and asks. /// - internal readonly struct ViiperStalePortSweep + internal readonly struct ViiperImportObservation { - private ViiperStalePortSweep(bool cleared, string reason) + private ViiperImportObservation(bool observed, string reason) { - Cleared = cleared; + Observed = observed; Reason = reason; } - public bool Cleared { get; } + public bool Observed { get; } - /// Why absence could not be established; null when it was. + /// Why the port list could not be read; null when it was. public string Reason { get; } - public static ViiperStalePortSweep Clear() => - new ViiperStalePortSweep(true, null); + public static ViiperImportObservation Seen() => + new ViiperImportObservation(true, null); - public static ViiperStalePortSweep Unproven(string reason) => - new ViiperStalePortSweep(false, reason); + public static ViiperImportObservation Unobserved(string reason) => + new ViiperImportObservation(false, reason); } internal static class ViiperUsbipPortManager { - private static readonly string[] KnownViiperDeviceIds = - { - "054c:05c4", // DualShock 4 (VIIPER CUH-ZCT1x identity) - "054c:09cc", // DualShock 4 - "054c:0ce6", // DualSense - "054c:0df2", // DualSense Edge - "045e:028e", // Xbox 360 - "057e:2069", // Switch 2 Pro - }; - private static readonly object ActivePortsLock = new object(); private static readonly HashSet ActivePorts = new HashSet(); /// - /// Sweeps stale local VIIPER imports and reports whether the machine was - /// afterwards proven free of them. + /// Reads the imported usbip ports, reports — without touching — any + /// local import this session does not manage, and says whether the + /// list could be read at all. /// - /// The distinction matters more than the sweep does. If every - /// usbip port query fails, the loop sees no ports, concludes nothing was - /// detached, and would otherwise report a clean window — turning "could not - /// look" into "looked and saw nothing". A caller about to create a device needs - /// those told apart, because only one of them is evidence. + /// Why this observes instead of detaching. Until + /// 2026-07-31 this was a sweep that detached every localhost import + /// carrying a known controller VID/PID and not registered by this + /// process. That identity test cannot tell a stale leftover from + /// another application's live virtual pad — both are a controller + /// VID/PID behind usbip://localhost — and on the first evening + /// two such applications ran side by side, it disconnected the other + /// one's DualSense mid-game. No evidence available here can make that + /// call: the serving backend knows which imports are its own, but an + /// import's consumer is not part of the usbip link at all. So + /// unmanaged imports are named in the log, pointed at the Settings + /// backend-process card (which can attribute and act with consent), + /// and left alone. + /// + /// What still refuses. The 3.3 rule is kept: a port list + /// that could not be read is not a port list that was empty. Callers + /// gating creation treat "could not look" as a refusal, exactly as + /// before. + /// + /// What no longer blocks. Unmanaged imports do not block + /// creation. Every device this session creates gets a fresh bus from + /// bus/create and its own import, so a foreign import is not a + /// reuse hazard (invariant (f) concerns our device's unproven + /// removal, and our removals are transactional in-session: the + /// lifetime object that created a port detaches that exact port). + /// /// - public static ViiperStalePortSweep DetachStaleLocalViiperPorts() + public static ViiperImportObservation ObserveLocalImports() { HashSet activePorts; lock (ActivePortsLock) @@ -5191,17 +5234,7 @@ public static ViiperStalePortSweep DetachStaleLocalViiperPorts() activePorts = new HashSet(ActivePorts); } - // USB/IP and PnP update asynchronously. A second stale import can - // become visible more than half a second after the first detach, so - // require a sustained clean window before input enumeration starts. - int cleanSnapshots = 0; - // A sustained clean window is required only when no device from - // this process owns a port (startup/crash recovery). Creating or - // removing a temporary companion while a native output is active - // can use one clean snapshot; registered ports protect the native - // device and PnP is already established. - int requiredCleanSnapshots = activePorts.Count > 0 ? 1 : 10; - // With usbip.exe absent every snapshot fails the same way; report + // With usbip.exe absent every attempt fails the same way; report // that once after the loop, not once per attempt. int failedQueries = 0; string lastQueryError = null; @@ -5211,78 +5244,104 @@ void RecordQueryFailure(string error) lastQueryError = error; } - int observedSnapshots = 0; - int staleRemaining = 0; - for (int attempt = 0; attempt < 32 && cleanSnapshots < requiredCleanSnapshots; attempt++) + bool observed = false; + for (int attempt = 0; attempt < 5 && !observed; attempt++) { - bool detachedAny = false; int queryFailuresBefore = failedQueries; - int staleThisSnapshot = 0; - foreach (UsbipPortBlock port in GetImportedPorts(RecordQueryFailure)) + IReadOnlyList ports = + GetImportedPorts(RecordQueryFailure); + if (failedQueries == queryFailuresBefore) { - if (!activePorts.Contains(port.Port) && - IsLocalViiperPort(port, null)) + observed = true; + string unmanaged = DescribeUnmanagedLocalImports(ports, + activePorts); + if (unmanaged != null) { - staleThisSnapshot++; - DetachPort(port.Port, - "stale local VIIPER controller import"); - detachedAny = true; + AppLogger.LogToGui(unmanaged, false); } } - - // Only a snapshot whose query actually succeeded is evidence about - // what is imported; a failed query says nothing either way. - if (failedQueries == queryFailuresBefore) - { - observedSnapshots++; - staleRemaining = staleThisSnapshot; - } - - cleanSnapshots = detachedAny ? 0 : cleanSnapshots + 1; - if (cleanSnapshots < requiredCleanSnapshots) + else if (attempt < 4) { Thread.Sleep(100); } } WarnPortQueryFailures(failedQueries, lastQueryError); - - return DecideStaleSweep(observedSnapshots, cleanSnapshots, - requiredCleanSnapshots, staleRemaining, lastQueryError); + return DecideImportObservation(observed, lastQueryError); } /// - /// Turns the sweep's observations into a verdict. Pure, so the rule that - /// matters — an unobserved machine is not a clean one — is testable without - /// a usbip client. + /// Turns the attempt history into a verdict. Pure, so the rule that + /// matters — an unobserved machine is not a clean one — is testable + /// without a usbip client. /// - /// Snapshots whose port query actually - /// succeeded. Zero means nothing was ever seen, however many attempts ran. - internal static ViiperStalePortSweep DecideStaleSweep(int observedSnapshots, - int cleanSnapshots, int requiredCleanSnapshots, int staleRemaining, - string lastQueryError) + internal static ViiperImportObservation DecideImportObservation( + bool observed, string lastQueryError) { - if (observedSnapshots <= 0) + if (!observed) { - // Every query failed. The loop saw no ports and detached nothing, which - // looks identical to a clean machine and is not the same thing. - return ViiperStalePortSweep.Unproven( + // Every query failed. Seeing no ports because nothing could be + // seen looks identical to a clean machine and is not the same + // thing. + return ViiperImportObservation.Unobserved( "the imported-port list could not be read" + (string.IsNullOrWhiteSpace(lastQueryError) ? string.Empty : " (" + lastQueryError.Trim() + ")")); } - if (cleanSnapshots < requiredCleanSnapshots) + return ViiperImportObservation.Seen(); + } + + /// + /// One log line naming every local import this session does not + /// manage, or null when there are none. Pure. The line must say what + /// was found, that it was deliberately left, and where the user can + /// act on it — it is the only trace this decision leaves. + /// + internal static string DescribeUnmanagedLocalImports( + IReadOnlyList ports, HashSet activePorts) + { + if (ports == null || ports.Count == 0) { - return ViiperStalePortSweep.Unproven( - "stale local VIIPER imports were still present after every attempt (" + - staleRemaining + " left at the last look)"); + return null; + } + + List unmanaged = new List(); + foreach (UsbipPortBlock port in ports) + { + if (!activePorts.Contains(port.Port) && IsLocalImport(port)) + { + unmanaged.Add(port.Port); + } } - return ViiperStalePortSweep.Clear(); + if (unmanaged.Count == 0) + { + return null; + } + + return string.Format(CultureInfo.InvariantCulture, + "{0} local usbip import(s) present that {1} does not manage (port {2}): left untouched - " + + "they may belong to another application or to a backend from a previous session. " + + "If one is a leftover virtual pad, Settings > VIIPER Virtual Controller Support > Backend process can clear it.", + unmanaged.Count, ProductInfo.ProductName, + string.Join(", ", unmanaged)); } + /// + /// Finds the usbip port of the device this session just created, by + /// its exact {busId}-{devId} bus id on a localhost server. + /// + /// Ambiguity fails rather than guesses: usbip bus ids are small + /// integers every server counts from the bottom, so two local servers + /// can both be serving a "1-7" — and adopting the wrong one means + /// detaching another application's pad at teardown. Two matches + /// therefore return -1, which rolls the creation back, instead of a + /// coin flip. Callers on a backend recent enough to report + /// usbipPort in the create response never get here at all. + /// + /// public static int FindLocalViiperPort(uint busId, string devId) { string remoteBusId = $"{busId}-{devId}"; @@ -5298,12 +5357,22 @@ void RecordQueryFailure(string error) { for (int attempt = 0; attempt < 15; attempt++) { - foreach (UsbipPortBlock port in GetImportedPorts(RecordQueryFailure)) + IReadOnlyList ports = + GetImportedPorts(RecordQueryFailure); + int match = SelectUniqueLocalBusidMatch(ports, remoteBusId, + out int matchCount); + if (match >= 0) { - if (IsLocalViiperPort(port, remoteBusId)) - { - return port.Port; - } + return match; + } + + if (matchCount > 1) + { + AppLogger.LogToGui(string.Format( + CultureInfo.InvariantCulture, + "VIIPER could not identify the import for {0}: {1} local imports claim that bus id, and adopting the wrong one would detach another application's device.", + remoteBusId, matchCount), true); + return -1; } if (attempt < 14) @@ -5320,6 +5389,43 @@ void RecordQueryFailure(string error) } } + /// + /// The port whose block matches on a + /// localhost server, or -1 when there is none or more than one. Pure. + /// + internal static int SelectUniqueLocalBusidMatch( + IReadOnlyList ports, string remoteBusId, + out int matchCount) + { + matchCount = 0; + int found = -1; + if (ports == null) + { + return -1; + } + + foreach (UsbipPortBlock port in ports) + { + if (IsLocalImport(port) && MatchesBusId(port, remoteBusId)) + { + matchCount++; + found = port.Port; + } + } + + return matchCount == 1 ? found : -1; + } + + /// + /// Detaches surviving older imports of the device this session just + /// created — and only of that device, on that device's own server. + /// + /// The server scope is what makes this safe to keep as a detach: + /// a candidate must share the usbip://host:port/ prefix of the + /// import we just confirmed as ours, because a bus id alone can + /// collide across two local servers. No confirmed import, no prefix, + /// no detaching. + /// public static void DetachDuplicateLocalViiperPorts(uint busId, string devId, int keepPort) { if (keepPort < 0) @@ -5328,13 +5434,78 @@ public static void DetachDuplicateLocalViiperPorts(uint busId, string devId, int } string remoteBusId = $"{busId}-{devId}"; - foreach (UsbipPortBlock port in GetImportedPorts()) + IReadOnlyList ports = GetImportedPorts(); + foreach (int duplicate in SelectSameServerDuplicates(ports, + remoteBusId, keepPort)) + { + DetachPort(duplicate, + $"duplicate import of this session's device {remoteBusId} on its own server"); + } + } + + /// + /// Ports other than that carry the same + /// bus id on the same server as . Pure. + /// Returns nothing when the kept port's own block — the source of the + /// server identity — cannot be found. + /// + internal static IReadOnlyList SelectSameServerDuplicates( + IReadOnlyList ports, string remoteBusId, + int keepPort) + { + List duplicates = new List(); + if (ports == null) + { + return duplicates; + } + + string serverPrefix = null; + foreach (UsbipPortBlock port in ports) + { + if (port.Port == keepPort) + { + serverPrefix = ExtractServerPrefix(port); + break; + } + } + + if (serverPrefix == null) + { + return duplicates; + } + + foreach (UsbipPortBlock port in ports) { - if (port.Port != keepPort && IsLocalViiperPort(port, remoteBusId)) + if (port.Port != keepPort && + MatchesBusId(port, remoteBusId) && + string.Equals(ExtractServerPrefix(port), serverPrefix, + StringComparison.OrdinalIgnoreCase)) { - DetachPort(port.Port, $"duplicate local VIIPER import for {remoteBusId}"); + duplicates.Add(port.Port); } } + + return duplicates; + } + + /// + /// The usbip://host:port/ part of a port block's device URL, or + /// null when the block does not carry one. + /// + internal static string ExtractServerPrefix(UsbipPortBlock port) + { + string block = port.Block; + int start = block.IndexOf("usbip://", + StringComparison.OrdinalIgnoreCase); + if (start < 0) + { + return null; + } + + int pathStart = block.IndexOf('/', start + "usbip://".Length); + return pathStart < 0 + ? null + : block.Substring(start, pathStart - start + 1); } public static void RegisterActivePort(int port) @@ -5472,24 +5643,40 @@ internal static string DescribePortQueryFailures(int attempts, string lastError) : $"VIIPER could not query usbip ports ({attempts} attempts): {lastError}"; } - private static bool IsLocalViiperPort(UsbipPortBlock port, string remoteBusId) + /// + /// Whether the import is served from this machine. Loopback origin is + /// the only identity test left in this class: what a local import + /// is — ours, another application's, a dead session's — cannot + /// be decided from the port table, and the controller-VID/PID + /// heuristic that used to stand in for that answer is how another + /// application's live pad got detached on 2026-07-31. + /// + internal static bool IsLocalImport(UsbipPortBlock port) { string block = port.Block.ToLowerInvariant(); - bool localHost = block.Contains("usbip://localhost:") || + return block.Contains("usbip://localhost:") || block.Contains("usbip://127.0.0.1:") || block.Contains("usbip://[::1]:") || block.Contains("usbip://::1:"); - bool busMatches = string.IsNullOrEmpty(remoteBusId) || - block.Contains("/" + remoteBusId.ToLowerInvariant()); - - return localHost && busMatches && (IsKnownViiperDevice(block) || !string.IsNullOrEmpty(remoteBusId)); } - private static bool IsKnownViiperDevice(string block) + internal static bool MatchesBusId(UsbipPortBlock port, string remoteBusId) { - foreach (string deviceId in KnownViiperDeviceIds) + if (string.IsNullOrEmpty(remoteBusId)) + { + return false; + } + + string block = port.Block.ToLowerInvariant(); + string needle = "/" + remoteBusId.ToLowerInvariant(); + for (int index = block.IndexOf(needle, StringComparison.Ordinal); + index >= 0; + index = block.IndexOf(needle, index + 1, StringComparison.Ordinal)) { - if (block.Contains(deviceId)) + // "/1-7" must not accept "/1-71": the bus id has to end where + // the match ends, not run on into more digits. + int after = index + needle.Length; + if (after >= block.Length || !char.IsLetterOrDigit(block[after])) { return true; } @@ -5608,7 +5795,10 @@ private static string FindUsbipPath() return null; } - private readonly struct UsbipPortBlock + // Internal rather than private: the pure attribution rules above are + // functions of these blocks, and the tests that pin them down build + // blocks directly. + internal readonly struct UsbipPortBlock { public UsbipPortBlock(int port, string block) { @@ -5629,14 +5819,14 @@ internal sealed class ViiperVirtualDeviceLifetime : IDisposable private readonly Action detachPort; private readonly Action unregisterPort; private readonly Action removeDevice; - private readonly Action detachStalePorts; + private readonly Action observeImports; private int disposed; internal ViiperVirtualDeviceLifetime(uint busId, string devId, int usbipPort, Action removeDevice, Action detachPort = null, Action unregisterPort = null, - Action detachStalePorts = null) + Action observeImports = null) { this.busId = busId; this.devId = devId ?? throw new ArgumentNullException(nameof(devId)); @@ -5645,10 +5835,13 @@ internal ViiperVirtualDeviceLifetime(uint busId, string devId, this.detachPort = detachPort ?? ViiperUsbipPortManager.DetachPort; this.unregisterPort = unregisterPort ?? ViiperUsbipPortManager.UnregisterActivePort; - // The sweep now reports whether absence was proven; this teardown path - // only needs it attempted, so the result is deliberately discarded here. - this.detachStalePorts = detachStalePorts ?? - (() => ViiperUsbipPortManager.DetachStaleLocalViiperPorts()); + // Observation, not a sweep: after this lifetime detaches its own + // port, the pass names anything local still imported — including + // this very port, if the detach above failed — without touching + // it. The verdict is deliberately discarded; teardown has nothing + // to refuse. + this.observeImports = observeImports ?? + (() => ViiperUsbipPortManager.ObserveLocalImports()); // This object's lifetime *is* our claim on the device, so it is // also the record of it. The exit-time backend stop reads the @@ -5699,7 +5892,7 @@ public void Dispose() try { - detachStalePorts?.Invoke(); + observeImports?.Invoke(); } catch { diff --git a/DS4WindowsTests/ViiperImportObservationTests.cs b/DS4WindowsTests/ViiperImportObservationTests.cs new file mode 100644 index 0000000..354edca --- /dev/null +++ b/DS4WindowsTests/ViiperImportObservationTests.cs @@ -0,0 +1,272 @@ +/* +Thrum +Copyright (C) 2026 Thrum contributors + +This program is free software: you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation, either version 3 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program. If not, see . +*/ + +using System; +using System.Collections.Generic; +using System.Linq; +using DS4Windows; + +namespace DS4WindowsTests; + +/// +/// The usbip import pass: what it may conclude, and — above all — what it may +/// touch. +/// +/// Two rules carry the safety here. First, 3.3's rule survives the +/// rewrite: a machine nobody could look at is not a machine known to be clean, +/// so an unreadable port list still refuses device creation. Second, the rule +/// the 2026-07-31 incident forced: an import this session did not create +/// cannot be attributed from the port table — a controller VID/PID behind a +/// localhost URL describes another application's live pad exactly as well as +/// a dead session's leftover, and the sweep that assumed otherwise +/// disconnected a live one mid-game. So attribution now needs the exact bus +/// id, on the same server, uniquely; everything weaker is reported and left +/// alone. +/// +[TestClass] +public class ViiperImportObservationTests +{ + private static ViiperUsbipPortManager.UsbipPortBlock Block(int port, + string url) => + new ViiperUsbipPortManager.UsbipPortBlock(port, + $"Port {port}: device in use at Full Speed(12Mbps)\n" + + $" unknown vendor : unknown product (054c:0ce6)\n" + + $" 7-{port} -> {url}\n"); + + // ---- The verdict: could the machine be looked at ---------------------- + + [TestMethod] + public void AMachineNobodyCouldLookAtIsNotProvenClean() + { + ViiperImportObservation observation = + ViiperUsbipPortManager.DecideImportObservation(false, + "usbip.exe was not found"); + + Assert.IsFalse(observation.Observed); + StringAssert.Contains(observation.Reason, "could not be read"); + StringAssert.Contains(observation.Reason, "usbip.exe was not found"); + } + + [TestMethod] + public void AnUnreadablePortListStillRefusesWhenThereIsNoErrorText() + { + ViiperImportObservation observation = + ViiperUsbipPortManager.DecideImportObservation(false, null); + + Assert.IsFalse(observation.Observed); + StringAssert.Contains(observation.Reason, "could not be read"); + } + + [TestMethod] + public void OneSuccessfulReadIsAnObservation() + { + ViiperImportObservation observation = + ViiperUsbipPortManager.DecideImportObservation(true, null); + + Assert.IsTrue(observation.Observed); + Assert.IsNull(observation.Reason); + } + + [TestMethod] + public void TheResultTypeCarriesItsOwnContract() + { + Assert.IsTrue(ViiperImportObservation.Seen().Observed); + Assert.IsNull(ViiperImportObservation.Seen().Reason); + Assert.IsFalse(ViiperImportObservation.Unobserved("why").Observed); + Assert.AreEqual("why", ViiperImportObservation.Unobserved("why").Reason); + } + + // ---- The log line for what was deliberately left alone ---------------- + + [TestMethod] + public void UnmanagedLocalImportsAreNamedAndPointedAtTheCard() + { + string line = ViiperUsbipPortManager.DescribeUnmanagedLocalImports( + new[] + { + Block(1, "usbip://localhost:3240/1-7"), + Block(3, "usbip://127.0.0.1:3241/1-2"), + }, + new HashSet()); + + Assert.IsNotNull(line); + StringAssert.Contains(line, "2 local usbip import(s)"); + StringAssert.Contains(line, "port 1, 3"); + StringAssert.Contains(line, "left untouched", + "The line is the only trace of the decision not to act; it has " + + "to say that the leaving was deliberate."); + StringAssert.Contains(line, "Backend process", + "A user with a leftover needs the path to the affordance that " + + "can actually clear it."); + } + + [TestMethod] + public void ThisSessionsOwnPortsAreNotReportedAsUnmanaged() + { + string line = ViiperUsbipPortManager.DescribeUnmanagedLocalImports( + new[] { Block(2, "usbip://localhost:3240/1-7") }, + new HashSet { 2 }); + + Assert.IsNull(line); + } + + [TestMethod] + public void RemoteImportsAreNoneOfOurBusiness() + { + string line = ViiperUsbipPortManager.DescribeUnmanagedLocalImports( + new[] { Block(4, "usbip://192.168.1.50:3240/1-1") }, + new HashSet()); + + Assert.IsNull(line, + "An import from another machine was chosen by the user; naming " + + "it as something this app 'does not manage' would only alarm."); + } + + [TestMethod] + public void AnEmptyPortTableSaysNothing() + { + Assert.IsNull(ViiperUsbipPortManager.DescribeUnmanagedLocalImports( + Array.Empty(), + new HashSet())); + Assert.IsNull(ViiperUsbipPortManager.DescribeUnmanagedLocalImports( + null, new HashSet())); + } + + // ---- Attribution: exact bus id, same server, uniquely ----------------- + + [TestMethod] + public void TheLocalhostFormsAllCountAsLocal() + { + Assert.IsTrue(ViiperUsbipPortManager.IsLocalImport( + Block(1, "usbip://localhost:3240/1-7"))); + Assert.IsTrue(ViiperUsbipPortManager.IsLocalImport( + Block(1, "usbip://127.0.0.1:3240/1-7"))); + Assert.IsTrue(ViiperUsbipPortManager.IsLocalImport( + Block(1, "usbip://[::1]:3240/1-7"))); + Assert.IsFalse(ViiperUsbipPortManager.IsLocalImport( + Block(1, "usbip://192.168.1.50:3240/1-7"))); + } + + [TestMethod] + public void ABusIdMatchesExactlyOrNotAtAll() + { + var block = Block(1, "usbip://localhost:3240/1-71"); + + Assert.IsTrue(ViiperUsbipPortManager.MatchesBusId(block, "1-71")); + Assert.IsFalse(ViiperUsbipPortManager.MatchesBusId(block, "1-7"), + "\"/1-7\" is a prefix of \"/1-71\"; accepting it would attribute " + + "somebody else's import to this device."); + Assert.IsFalse(ViiperUsbipPortManager.MatchesBusId(block, null)); + Assert.IsFalse(ViiperUsbipPortManager.MatchesBusId(block, "")); + } + + [TestMethod] + public void AUniqueLocalBusidMatchIsAdopted() + { + int port = ViiperUsbipPortManager.SelectUniqueLocalBusidMatch( + new[] + { + Block(1, "usbip://localhost:3240/1-7"), + Block(2, "usbip://localhost:3240/1-8"), + }, + "1-7", out int matches); + + Assert.AreEqual(1, port); + Assert.AreEqual(1, matches); + } + + /// + /// The collision the uniqueness rule exists for: usbip bus ids are small + /// integers every server counts from the bottom, so two local servers can + /// both be serving a "1-7". Guessing means adopting — and at teardown, + /// detaching — another application's device. + /// + [TestMethod] + public void AnAmbiguousBusidMatchRefusesToGuess() + { + int port = ViiperUsbipPortManager.SelectUniqueLocalBusidMatch( + new[] + { + Block(1, "usbip://localhost:3240/1-7"), + Block(2, "usbip://localhost:3241/1-7"), + }, + "1-7", out int matches); + + Assert.AreEqual(-1, port); + Assert.AreEqual(2, matches); + } + + [TestMethod] + public void ARemoteImportNeverMatchesLocalAttribution() + { + int port = ViiperUsbipPortManager.SelectUniqueLocalBusidMatch( + new[] { Block(1, "usbip://192.168.1.50:3240/1-7") }, + "1-7", out int matches); + + Assert.AreEqual(-1, port); + Assert.AreEqual(0, matches); + } + + [TestMethod] + public void DuplicateDetachIsScopedToTheConfirmedImportsOwnServer() + { + var ports = new[] + { + Block(1, "usbip://localhost:3240/1-7"), // ours, confirmed + Block(2, "usbip://localhost:3240/1-7"), // duplicate, same server + Block(3, "usbip://localhost:3241/1-7"), // same bus id, other server + }; + + IReadOnlyList duplicates = + ViiperUsbipPortManager.SelectSameServerDuplicates(ports, "1-7", 1); + + CollectionAssert.AreEqual(new List { 2 }, duplicates.ToList(), + "Port 3 carries the same bus id on a different server - that is " + + "another program's device, not our duplicate."); + } + + [TestMethod] + public void NoConfirmedImportMeansNoDuplicateDetaching() + { + var ports = new[] + { + Block(2, "usbip://localhost:3240/1-7"), + }; + + Assert.AreEqual(0, ViiperUsbipPortManager.SelectSameServerDuplicates( + ports, "1-7", 1).Count, + "The kept port's own block is the source of the server identity; " + + "without it there is no scope, and no scope means no detaching."); + Assert.AreEqual(0, ViiperUsbipPortManager.SelectSameServerDuplicates( + null, "1-7", 1).Count); + } + + [TestMethod] + public void TheServerPrefixIsTheUrlUpToThePath() + { + Assert.AreEqual("usbip://localhost:3240/", + ViiperUsbipPortManager.ExtractServerPrefix( + Block(1, "usbip://localhost:3240/1-7"))); + Assert.AreEqual("usbip://[::1]:3240/", + ViiperUsbipPortManager.ExtractServerPrefix( + Block(1, "usbip://[::1]:3240/1-7"))); + Assert.IsNull(ViiperUsbipPortManager.ExtractServerPrefix( + new ViiperUsbipPortManager.UsbipPortBlock(1, + "Port 1: something without a device URL"))); + } +} diff --git a/DS4WindowsTests/ViiperStalePortSweepTests.cs b/DS4WindowsTests/ViiperStalePortSweepTests.cs deleted file mode 100644 index 1d8dd7e..0000000 --- a/DS4WindowsTests/ViiperStalePortSweepTests.cs +++ /dev/null @@ -1,123 +0,0 @@ -/* -Thrum -Copyright (C) 2026 Thrum contributors - -This program is free software: you can redistribute it and/or modify -it under the terms of the GNU General Public License as published by -the Free Software Foundation, either version 3 of the License, or -(at your option) any later version. - -This program is distributed in the hope that it will be useful, -but WITHOUT ANY WARRANTY; without even the implied warranty of -MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -GNU General Public License for more details. - -You should have received a copy of the GNU General Public License -along with this program. If not, see . -*/ - -using DS4Windows; - -namespace DS4WindowsTests; - -/// -/// The stale-import sweep's verdict (plan task 3.3, lifecycle invariant (f): -/// "unproven removal blocks reuse"). -/// -/// The rule under test is the one the invariant actually turns on: a machine -/// nobody could look at is not a machine known to be clean. Before this, every -/// usbip port query could fail, the loop would see no ports, conclude nothing -/// needed detaching, and report the same clean window it reports when it genuinely -/// looked and found nothing — so "could not look" silently became evidence of -/// absence, and device creation proceeded on it. -/// -[TestClass] -public class ViiperStalePortSweepTests -{ - [TestMethod] - public void AMachineNobodyCouldLookAtIsNotProvenClean() - { - // Zero observed snapshots: every query failed. cleanSnapshots reaching the - // requirement here is exactly the false negative - the loop counts a snapshot - // as clean when it detached nothing, and a failed query detaches nothing. - ViiperStalePortSweep sweep = ViiperUsbipPortManager.DecideStaleSweep( - observedSnapshots: 0, cleanSnapshots: 10, requiredCleanSnapshots: 10, - staleRemaining: 0, lastQueryError: "usbip.exe not found"); - - Assert.IsFalse(sweep.Cleared, - "no successful query means absence was never established."); - StringAssert.Contains(sweep.Reason, "could not be read"); - StringAssert.Contains(sweep.Reason, "usbip.exe not found", - "the refusal has to carry why, or the user cannot act on it."); - } - - [TestMethod] - public void AnUnreadablePortListStillRefusesWhenThereIsNoErrorText() - { - ViiperStalePortSweep sweep = ViiperUsbipPortManager.DecideStaleSweep( - observedSnapshots: 0, cleanSnapshots: 10, requiredCleanSnapshots: 10, - staleRemaining: 0, lastQueryError: " "); - - Assert.IsFalse(sweep.Cleared); - Assert.IsFalse(sweep.Reason.Contains("("), - "an empty error must not produce an empty parenthetical."); - } - - [TestMethod] - public void StaleImportsThatSurviveEveryAttemptRefuse() - { - ViiperStalePortSweep sweep = ViiperUsbipPortManager.DecideStaleSweep( - observedSnapshots: 32, cleanSnapshots: 0, requiredCleanSnapshots: 10, - staleRemaining: 2, lastQueryError: null); - - Assert.IsFalse(sweep.Cleared); - StringAssert.Contains(sweep.Reason, "still present"); - StringAssert.Contains(sweep.Reason, "2", - "how many were left is the difference between a hiccup and a stuck port."); - } - - [TestMethod] - public void AnObservedAndSustainedCleanWindowIsProof() - { - ViiperStalePortSweep sweep = ViiperUsbipPortManager.DecideStaleSweep( - observedSnapshots: 10, cleanSnapshots: 10, requiredCleanSnapshots: 10, - staleRemaining: 0, lastQueryError: null); - - Assert.IsTrue(sweep.Cleared, "looked, repeatedly, and saw nothing: that is proof."); - Assert.IsNull(sweep.Reason); - } - - [TestMethod] - public void OneObservedSnapshotIsEnoughWhenThatIsAllTheLoopRequired() - { - // requiredCleanSnapshots drops to 1 when this process already owns a port, - // because PnP is established and the native device is protected. - ViiperStalePortSweep sweep = ViiperUsbipPortManager.DecideStaleSweep( - observedSnapshots: 1, cleanSnapshots: 1, requiredCleanSnapshots: 1, - staleRemaining: 0, lastQueryError: null); - - Assert.IsTrue(sweep.Cleared); - } - - [TestMethod] - public void APartiallyObservedSweepIsJudgedOnWhatItSaw() - { - // Some queries failed and some succeeded. The successful ones are evidence, - // so a sustained clean window among them still counts - a transient failure - // must not permanently block device creation. - ViiperStalePortSweep sweep = ViiperUsbipPortManager.DecideStaleSweep( - observedSnapshots: 7, cleanSnapshots: 10, requiredCleanSnapshots: 10, - staleRemaining: 0, lastQueryError: "transient"); - - Assert.IsTrue(sweep.Cleared); - } - - [TestMethod] - public void TheResultTypeCarriesItsOwnContract() - { - Assert.IsTrue(ViiperStalePortSweep.Clear().Cleared); - Assert.IsNull(ViiperStalePortSweep.Clear().Reason); - Assert.IsFalse(ViiperStalePortSweep.Unproven("why").Cleared); - Assert.AreEqual("why", ViiperStalePortSweep.Unproven("why").Reason); - } -} diff --git a/docs/dev/PLAN-PROGRESS.md b/docs/dev/PLAN-PROGRESS.md index 581dc7f..b134b1d 100644 --- a/docs/dev/PLAN-PROGRESS.md +++ b/docs/dev/PLAN-PROGRESS.md @@ -3656,3 +3656,78 @@ names.** The stand-in server was a `powershell` process, not `viiper.exe`, and `GetExtendedTcpTable` identified it correctly by ownership of the listening socket — a name-matching implementation would have found nothing to stop, and a looser one would have gone looking for the wrong process entirely. + +## 2026-07-31 — Fix: the stale-port sweep detached another application's live controller + +**Session scope:** the follow-up filed at the end of the previous entry, fixed the same night +because it had already cost a real controller. Branch `fix/port-sweep-ownership`. Verdict for +(f) in [`lifecycle-invariants.md`](lifecycle-invariants.md) revised. + +### Defect origin + +The UI verification in the previous entry launched a Thrum build on the dev PC while the +maintainer's native-mode DS4Windows build was serving a virtual DualSense over usbip during a +game. Thrum's startup sweep detached it. The controller went dead mid-game; the log line +`VIIPER detached usbip port 1 (stale local VIIPER controller import)` was the only trace. + +### Mechanism + +`DetachStaleLocalViiperPorts` decided an import was a stale leftover of ours if it came from a +localhost usbip URL, carried a VID/PID in `KnownViiperDeviceIds`, and was not a port *this +process* had registered. Every one of those is also true of a different application's live +virtual pad. The premise was the bug: **a local import cannot be attributed from the port +table at all**, because the usbip link records the serving side and never the consumer, so a +dead session's leftover and a live consumer's device are the same row. + +Arriving with PR #28, not #29 — but #29 is where the irony lives, since its whole (d) story is +"a backend we did not start is not ours to stop" while this sweep had no ownership test at all. + +### The fix + +`ObserveLocalImports` replaces the sweep and detaches nothing. It reads the port table, names +any local import this session does not manage in one log line pointing at the (d) +backend-process card — which can attribute leftovers through the backend census and clear them +with consent — and leaves them alone. 3.3's fail-closed rule is untouched: an unreadable port +list still refuses creation, which is what `ViiperImportObservation` now carries. + +Two narrower attribution bugs surfaced while proving that, each able to detach somebody else's +device on its own: + +- `FindLocalViiperPort` matched our just-created device by bus id alone. usbip bus ids are + small integers every server counts from the bottom, so two local servers can both serve a + `1-7`, and the first hit wins — then teardown detaches it. It now refuses on ambiguity + (`-1`, rolling the creation back) rather than guessing, and the create response's + `usbipPort` (upstream `viipertypes.Device`, added 2026-07-30) is believed over any scan + when the backend reports it. Backends predating the field fall back to the scan. +- `DetachDuplicateLocalViiperPorts` is scoped to the `usbip://host:port/` prefix of the import + we confirmed as ours, so a same-bus-id device on a different local server is unreachable. + No confirmed import, no prefix, no detaching. + +`KnownViiperDeviceIds` is gone. Nothing in this class decides what an import *is* from its +controller identity any more; the only identity test left is whether the server is loopback. + +### What this gives up, deliberately + +Automatic cleanup of a genuine leftover from a hard-killed session. That becomes a consented +user action via the (d) card, which is the same trade (d) already makes and for the same +reason: the application cannot prove the thing is abandoned. The startup self-ingestion risk +the sweep originally guarded (ingesting our own virtual pad as an input) is accepted as +residual and called out at its call site in `ControlService`; the (d) startup warning fires on +exactly that state. + +### Verification + +Sixteen tests in `ViiperImportObservationTests` (replacing `ViiperStalePortSweepTests`, whose +verdict cases are carried over): the observation verdict, the report line's required content, +and the attribution rules — bus-id matching that rejects `/1-7` against `/1-71`, ambiguity +refusing to guess, remote imports never attributed locally, duplicate detach scoped to one +server, and no-confirmed-import meaning no detaching. + +**Reproduced against the original failure, live.** With the maintainer's native-mode +DS4Windows still serving its DualSense on port 1, the fixed build was launched isolated: +`usbip port` showed the import before and after, unchanged; four Sony devnodes stayed present; +the controller survived both Thrum start and shutdown. The log carried +`1 local usbip import(s) present that Thrum does not manage (port 1): left untouched ...` — +the report the detach became. + +Suite: **862 passed / 0 failed** (CI filter), from 853. diff --git a/docs/dev/lifecycle-invariants.md b/docs/dev/lifecycle-invariants.md index fbd680d..5e457fb 100644 --- a/docs/dev/lifecycle-invariants.md +++ b/docs/dev/lifecycle-invariants.md @@ -309,6 +309,41 @@ could not prove absence. > that already handle a refused creation handle this too. The verdict rules are extracted > into `DecideStaleSweep` and covered by seven tests. +> **Revised 2026-07-31: the sweep was reading a heuristic as ownership, and it cost a user +> their controller.** `DetachStaleLocalViiperPorts` identified "ours" as *any* import from a +> localhost usbip URL carrying a known controller VID/PID and not registered by this +> process. That is also an exact description of a **different application's live virtual +> pad**. On the first evening two such applications ran side by side — Thrum under test and +> the maintainer's native-mode DS4Windows build serving a DualSense — Thrum's startup sweep +> detached the other one's controller mid-game. +> +> The mistaken premise was that a local import can be attributed from the port table at all. +> It cannot: the usbip link records the *serving* side, never the consumer, so a dead +> session's leftover and a live consumer's device are the same row. The sweep is now +> `ObserveLocalImports` and detaches nothing. It reads the table, names any local import +> this session does not manage in one log line pointing at the (d) backend-process card — +> which *can* attribute leftovers, via the backend census, and clear them with consent — and +> leaves them alone. +> +> **What this costs (f), stated plainly.** The invariant's own subject is unaffected: "our +> device's removal is unproven" stays impossible, because our removals are transactional +> in-session — the lifetime object that created a port detaches that exact port — and every +> new device gets a fresh bus from `bus/create`, so a foreign import is not a reuse hazard. +> 3.3's fail-closed rule is kept intact: an unreadable port list still refuses creation. +> What is genuinely given up is *automatic* cleanup of a leftover from a session that died +> hard. That is now a consented user action rather than a silent one, which is the same +> trade (d) makes, and for the same reason: this application cannot prove the thing is +> abandoned. +> +> Two narrower attribution bugs surfaced while proving the above, both able to detach +> somebody else's device on their own. `FindLocalViiperPort` matched our just-created device +> by bus id alone, and usbip bus ids are small integers every server counts from the bottom, +> so two local servers can both serve a `1-7`; it now refuses on ambiguity (`-1`, rolling +> the creation back) instead of adopting the first hit, and prefers the `usbipPort` the +> backend reports in the create response over scanning at all. `DetachDuplicateLocalViiperPorts` +> is now scoped to the `usbip://host:port/` prefix of the import we confirmed as ours, so a +> same-bus-id device on a *different* local server is out of reach. Sixteen tests. + `CreateDeviceAndOpenStream` opens with: ```csharp @@ -342,7 +377,7 @@ directly in the invariant's spirit, and testable through the existing port-manag | (c) prove exact-device absence | **Present** at both levels — census since 3.1, PnP cross-check since 2026-07-31; fail-closed | done | | (d) parent death retains a protection | **N/A** — architecture prevents the dangerous case | done — unowned-backend diagnostics card + consented stop, 2026-07-31 | | (e) timeout ≠ permission to kill | **Present** via the census gate | None; document the check-then-kill window | -| (f) unproven removal blocks reuse | **Present** — since 3.3; also closed a fail-open where an unreadable port list counted as clean | done | +| (f) unproven removal blocks reuse | **Present** — since 3.3; unreadable port list still refuses. Revised 2026-07-31: the sweep no longer detaches imports it cannot attribute, after it disconnected another application's live pad | done | **The headline for Phase 3 planning: there is far less to port than the plan assumed.** The plan budgeted 3–6 sessions on the assumption that a large body of old-fork containment work would need