Stop detaching usbip imports we cannot prove are ours - #30
Merged
Conversation
The stale-import sweep decided an import was our leftover if it came from a localhost usbip URL, carried a controller VID/PID we recognise, and was not a port this process had registered. Every one of those is equally true of a different application's live virtual pad. On the first evening two such applications ran side by side -- a Thrum build under test and a native-mode DS4Windows serving a DualSense -- Thrum's startup sweep detached the other one's controller in the middle of a game. The premise was the defect. A local import cannot be attributed from the port table at all: 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. No amount of refining the VID/PID list fixes that. So ObserveLocalImports replaces the sweep and detaches nothing. It reads the table, names any local import this session does not manage in one log line pointing at the backend-process card -- which can attribute leftovers through the backend census and clear them with consent -- and leaves them alone. 3.3's rule is kept exactly: an unreadable port list is not an empty one, and still refuses creation. What is given up is automatic cleanup of a leftover from a hard-killed session, which becomes a consented action instead. That is the same trade invariant (d) already makes, for the same reason: this application cannot prove the thing is abandoned. Two narrower attribution bugs surfaced while proving the above, each able to detach somebody else's device on its 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 be serving a "1-7" -- the first hit won, and teardown then detached it. It now refuses on ambiguity, rolling the creation back, 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. KnownViiperDeviceIds is gone. Nothing here decides what an import is from its controller identity any more; the only identity test left is whether the server is loopback. Sixteen tests, and a live reproduction against the original failure: with the native-mode build still serving its DualSense on port 1, the fixed build was launched isolated and the import was unchanged before and after, through both start and shutdown. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the follow-up filed at the end of #29, the same night, because it had already cost a real controller.
What happened
The UI verification in #29 launched a Thrum build on the dev PC while a 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, and
VIIPER detached usbip port 1 (stale local VIIPER controller import)was the only trace.Mechanism
DetachStaleLocalViiperPortsdecided an import was our stale leftover if it:KnownViiperDeviceIds, andEvery one of those is equally true of a different application's live virtual pad.
The premise was the defect, not the heuristic's precision: a local import cannot be attributed from the port table at all. The usbip link records the serving side and never the consumer, so a dead session's leftover and a live consumer's device are literally the same row. No refinement of the VID/PID list fixes that.
Arriving with #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
ObserveLocalImportsreplaces 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 rule is kept exactly: an unreadable port list is not an empty one, and still refuses creation (
ViiperImportObservationcarries that verdict).Two narrower attribution bugs surfaced while proving the above, each able to detach somebody else's device on its own:
FindLocalViiperPortmatched by bus id alone. usbip bus ids are small integers every server counts from the bottom, so two local servers can both serve a1-7— the first hit won, and teardown then detached it. It now refuses on ambiguity (-1, rolling the creation back) rather than guessing, and prefers theusbipPortthe backend reports in the create response (upstreamviipertypes.Device, added 2026-07-30) over scanning at all. Older backends fall back to the scan.DetachDuplicateLocalViiperPortsis now scoped to theusbip://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. No confirmed import, no prefix, no detaching.KnownViiperDeviceIdsis deleted. 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 — the same trade (d) already makes, for the same reason: the application cannot prove the thing is abandoned. The startup self-ingestion risk the sweep originally guarded is accepted as residual and called out at its
ControlServicecall site; the (d) startup warning fires on exactly that state.Verification
Suite: 862 passed / 0 failed (CI filter), from 853.
Sixteen tests in
ViiperImportObservationTests(replacingViiperStalePortSweepTests, whose verdict cases carry over): the observation verdict, the report line's required content, and the attribution rules — bus-id matching that rejects/1-7against/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 native-mode DS4Windows still serving its DualSense on port 1, the fixed build was launched isolated:
usbip portshowed the import unchanged before and after, four Sony devnodes stayed present, and the controller survived both Thrum start and shutdown. The log carried the report the detach became:🤖 Generated with Claude Code