Export.{Snapshot,LeaseCheck,DeviceInfo}, ExportHost.{Reconcile,FinishImport},
ImportHost.Start, and all 11 exportLedger methods carried ctx params that
implementations never consumed (linux FinishImport now reaches into h.runCtx
internally). UrbTransaction.{Wait,Cancel} did consume ctx, but every call site
passed context.Background(), forcing the reverse pattern in endpoint_darwin
where e.ctx was already cancelled. Cancel becomes synchronous, Wait reads
under the close(t.done) happens-before. ExportHost's runCtx now derives in
newPlatformExportHost so Start() is just precondition-check (linux
ensureKernelPath / darwin no-op), and the three pre-Start nil defences in
Close/Events fall out.
- Delete docs/adr/0001 and every code reference; the immutability contract
is now a four-line comment on the Export interface where it applies.
- Inline applyStaleClones at the linux/darwin reconcile call sites and
drop the helper plus the parallel staleBusIDs/pendingByBusID maps the
darwin host built only to match its signature.
- Inline single-use sentinel errors in iso_scheduler.go and
endpoint_darwin.go (no errors.Is callers).
- Rename exportLedger.fast/inventory to broadcastAccess/inventoryAccess
to match the Access suffix used by every other lock in the package.
- Strip narrative doc comments on exportLedger, the with* helpers,
IssueLease, ConsumeLeaseAndReserve, TryReserveForImport, host.go
interfaces, cgoCallbackHandle, and cloneDarwinExport; keep only the
non-obvious WHY required by .claude/rules/code-comment.md.
Net: 8 files, 99 insertions / 291 deletions, no behaviour change.
ExportHost.Events spawned goroutines tied to a caller-supplied ctx, and
neither host.Close nor the per-error cleanup in ServerService.Start
cancelled that ctx. A bind failure on port 3240 after Events succeeded
leaked ueventLoop, the netlink fd, and the darwin watcher goroutine.
The host now owns its background lifecycle: Start derives runCtx from
the caller's ctx, Close cancels it, Events binds goroutines to it.
ServerService.Start switches to a deferred cleanup that cancels and
closes the host on any error, so future failure steps cannot forget it.
- Route every reserved-state mutation through withInventoryWrite; the
lease insert path now broadcasts so extended subscribers see the busy
transition immediately instead of waiting for the next mutation. Folds
cleanupExpiredLocked changes into the broadcast decision on every
IssueLease early-reject path. Rename the field to inventory and force
read/write/write-quiet sites through dedicated accessors so future
callers cannot bypass the invariant.
- Mirror the linux clone-then-swap pattern in darwinExportHost.Reconcile
via cloneDarwinExport, so the ledger's unlocked Snapshot / LeaseCheck
reads never observe a half-mutated stale flag. Documented as
docs/adr/0001-export-pointer-immutability.md and on the Export
interface; both hosts now share the applyStaleClones helper.
- Rewrite 27 if (_, )?err := …; err != nil sites to assign-then-check
per .claude/rules/go-syntax.md.
- Delete parse / builder / tautological tests forbidden by
.claude/rules/code-test.md (option/usbip_test.go,
iso_scheduler_test.go, usbhost_darwin_status_test.go).
- Tag nine more usbip files with linux || (darwin && cgo); fixes
pre-existing windows / android lint failures because the protocol
types were only consumed by tagged code.
Three review findings shared one shape: a contract documented in
comments but reimplemented at each call site. Each fix collapses
the contract into a single named home.
- RebaseFrame now rounds up to the least frame >= currentFrame
with the matching low 8 bits, so scheduled iso submits never
land in the past. The .m bridge trusts the asap flag instead
of treating start_frame=0 as ASAP, which also fixes a latent
int32-wraparound path.
- NewServerService and NewClientService reject empty devices
with a clear error instead of starting up exporting nothing.
- exportLedger.reservedLocked unifies busy + outstanding lease;
AvailableExports and snapshotDeviceState consult it so legacy
devlist and control snapshots no longer advertise a leased
busid as available. IsBusy renamed to IsReserved across the
ExportHost interface and call sites.
Two supporting deepenings:
- Drop FrameOracle/IsoScheduler; iso submit is now the free
EncodeIsoSubmit(currentFrame, ...). The darwin endpoint holds
currentFrame func() uint64.
- Extract SelectMatches; host_linux and host_darwin Reconcile
loops share the match/dedup step.
The peer now owns submit/cancel lifecycle: pendingSubmit retains the
original DevID per submit, CMD_UNLINK carries it (stub_rx valid_request
was silently rejecting devid=0), and RET_UNLINK finalizes the bound
transaction. UrbTransaction.Cancel delegates instead of assembling the
wire itself. Stale exports stay in linux and darwin host snapshots so
the ledger broadcasts unavailable State updates rather than Removed.
Three independent correctness bugs that all violated "decide before
publishing": each surface (wire reply, sysfs path, broadcast state)
committed to a state the code later had to contradict.
darwin: switch IOUSBHostPipe abort to IOUSBHostAbortOptionSynchronous and
add the EP-0 path via abortDeviceRequestsWithOption (previously a silent
no-op for control transfers). After abort returns, await a per-pending
drained channel before writing RET_UNLINK so the late RetSubmit
suppression in finishSubmit has actually taken effect — the drained
channel is allocated lazily by markSubmitUnlinked, so the hot submit
path is unchanged.
linux: collapse the per-controller VHCI abstraction. The kernel exposes
attach/detach/status/status.N only on vhci_hcd.0 with globally unique
port numbers; the previous code wrote vhci_hcd.N/attach for N>0 and
silently failed past the primary controller's port range. Glob status*
on the primary, key reservations on bare port int, and carry the
status-suffix as a diagnostic-only secondary index for Description().
export_ledger: ConsumeLeaseAndReserve no longer publishes busy=true and
then conditionally rolls it back. Read seq under fast first, then make
every decision (including the lease.Generation check) inside one slow
critical section before any busy mutation. This removes the window
where a concurrent BroadcastIfChanged could observe transient busy and
poison l.state with no corrective broadcast on rollback.
Address `golangci-lint` modernize and unused warnings:
- replace map copy loops with `maps.Copy`
- collapse `if x > y { x = y }` clamps to `min`/`max`
- switch `for i := 0; i < n; i++` to `for i := range n`
- use `t.Context()` in the relay handoff test
- delete the unused `removeOnce` field and `openBinaryDevice` helper
On Linux the server was closing the userspace conn inside
NewServerDataSession (handoff.Start closes h.conn synchronously in
direct-TCP mode), so the subsequent WriteOpRepImport always wrote to a
closed socket and every default-mode import failed. Split DataSession
into prepare + Start: the server now writes the reply on the live conn
and only then calls session.Start() to hand the wire to the kernel.
Close also closes h.conn now so the prepare-then-Close error path
doesn't leak the fd.
On Darwin FinishImport returned false after successfully re-capturing
a pending replacement for a stale busy export, so the server skipped
reconcileAndBroadcast and the ledger kept the device hidden until
another IOKit topology event. Return true to trigger reconcile; the
next Reconcile fast-paths the matching registryID back into the
snapshot.
Apply the <10-LoC / <3-non-test-caller heuristic across the package:
inline writeControlFrame, removeLeaseWaiter, vhciAttach, vhciHubForSpeed,
applyControlSnapshot, applyRemoteEntries, shouldRetryBusID, entrySerial,
and the trivial darwin/linux import-host constructors at their call
sites.
Delete the darwinClientSession wrapper -- darwinVirtualController already
satisfies DataSession, so attach the Description() method to it directly
and return the controller from darwinImportHost.Attach.
Fold the 11-line data_session.go into host.go beside AttachedSession,
which embeds DataSession.
Inline thin pass-through helpers (< 10 lines, < 3 non-test callers) and
the writeSysfs path wrappers; delete the test-only vhciUsedPorts plus
its singleflight machinery and the never-called clientAssignment.IsActive.
Remove four private single-impl interfaces (usbEventListener,
darwinUSBHostDeviceWatch, darwinServerDataDevice, darwinEndpointStateMachine)
and both function-table DI seams (usbipOps, darwinServerOps); production calls
the underlying functions directly. Inline ~25 single-call helpers and fuse
subordinate routines into their sole caller. Collapse single-field helper
structs into their underlying types. Fold server_linux.go/server_darwin.go
into host_linux.go/host_darwin.go.
Delete change-detector tests per .claude/rules/code-test.md: parse round-trip,
mirror, input-validation, and builder-property suites that never exercised
real syscalls, network, or cgo. Six test files removed; linux_test.go slimmed
to three real-system tests; darwin_integration_test.go retains the six
real-cgo tests. Surviving suite: 16 tests that all exercise real OS APIs or
spawn the official Linux usbip server.
Net: -5100 LoC (~37% of the package).
The ExportHost.Reconcile changed bool is computed by both Linux and
Darwin implementations but discarded at the only call site
(reconcileAndBroadcast); the ledger already detects change in
BroadcastIfChanged. linuxImportHost.trackPort had a dead add=true
branch — all callers passed false. darwinExport.Snapshot was setting
StatusReason to the same string as State (tautological). LeaseCheck
for stale captures returned the State name as the reason.
Also trim narrative comments in export_ledger.go that referenced
refactor history or the deleted lease.go file, inline the trivial
checkAvailableUnderLock helper at its only caller, and replace
cloneIsoPackets with slices.Clone.
Collapse ServerService and ClientService to one definition each by
hiding Linux vs Darwin behind ExportHost, ImportHost, and Export
seams. Along the way, extract three state machines that previously
lived as scattered fields on the service structs:
- LeaseManager owns its own mutex and closes the
availability-vs-insert TOCTOU by checking export busy inside Issue
under the same lock as the insert.
- DataSession gives the three per-import data-plane implementations
(Linux kernel handoff, Darwin server data session, Darwin virtual
controller) a uniform Done/Err/Close interface.
- clientAssignment encapsulates the matched/import-all target state
and exposes ApplyMatched/ApplyAll diffs to ClientService, which
keeps worker goroutine lifecycle.
Service busy tracking moves off the per-platform serverExport struct
onto ServerService.busy, since it follows the lease/import lifecycle
rather than physical claim/release. linux_test.go is migrated to
construct ServerService and ClientService through the new host
interfaces.
世界