usbip: trim ADR + narrative comments, rename locks, drop one-use helpers
- 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.
This commit is contained in:
世界
@@ -1,76 +0,0 @@
|
||||
# ADR-0001 — Export pointer immutability
|
||||
|
||||
- Status: Accepted
|
||||
- Date: 2026-05-16
|
||||
- Scope: `service/usbip/host.go`, `service/usbip/host_linux.go`,
|
||||
`service/usbip/host_darwin.go`, `service/usbip/export_ledger.go`
|
||||
|
||||
## Context
|
||||
|
||||
The `Export` interface (`service/usbip/host.go`) is the seam between
|
||||
platform `ExportHost` implementations and the platform-neutral
|
||||
`exportLedger`. The ledger publishes Export pointers to control
|
||||
subscribers and import sessions, and it calls `Snapshot`,
|
||||
`LeaseCheck`, `LeaseIdentity`, and `DeviceInfo` on those pointers
|
||||
**outside** any lock (`export_ledger.go` ~ lines 190, 274, 377, 592).
|
||||
Holding the inventory lock across these calls is not viable because
|
||||
they may perform syscalls (Linux re-reads `usbip_status`; Darwin
|
||||
re-reads IOKit state).
|
||||
|
||||
Two implementation paths existed:
|
||||
|
||||
1. Each host carries a per-Export lock and `Snapshot` etc. acquire it.
|
||||
Cost: every method on every Export takes a lock. The lock has to
|
||||
live in the platform struct (the interface is read-only). Three
|
||||
call sites in the ledger × N hosts. Easy to forget on a new host.
|
||||
2. Hosts treat published Export pointers as immutable. Mutation is
|
||||
"clone the published pointer, mutate the clone, swap it into the
|
||||
host's committed map under the host's lock". The ledger's
|
||||
unlocked reads are safe because the reader holds a pointer that
|
||||
nobody mutates.
|
||||
|
||||
Linux already followed pattern 2 by convention (`cloneLinuxExport` +
|
||||
the `committed` map swap in `Reconcile`). Darwin did not — its
|
||||
`Reconcile` set `exp.stale = true` and `exp.pendingRegistryID = …`
|
||||
on the same `*darwinExport` value the ledger had already received
|
||||
(`host_darwin.go:177-180`). Under `go test -race` this is a clean
|
||||
data race; in production it can publish an inconsistent tuple
|
||||
(`stale=true`, `pendingRegistryID=0`) if writes reorder.
|
||||
|
||||
## Decision
|
||||
|
||||
Pattern 2 is the contract. Published Export pointers are immutable
|
||||
from the ledger's perspective. Hosts that need to change Export
|
||||
state MUST clone, mutate the clone, then swap the clone into their
|
||||
committed map under their own lock. The previously-handed-out pointer
|
||||
is never written to.
|
||||
|
||||
The contract is documented on the `Export` interface declaration
|
||||
itself. A shared helper `applyStaleClones` (`host.go`) enforces the
|
||||
clone-then-mark rule for the common "mark a busid stale" transition;
|
||||
both Linux and Darwin reconcile flows use it.
|
||||
|
||||
## Consequences
|
||||
|
||||
- The ledger reads `Snapshot/LeaseCheck/…` outside its inventory lock
|
||||
without per-Export locking and without races. This keeps the
|
||||
inventory lock fast and avoids re-entering hosts under it.
|
||||
- Hosts pay a small allocation per stale/replace transition (one
|
||||
struct value copy plus a `slices.Clone` of any embedded slice).
|
||||
Reconcile is not on the hot path.
|
||||
- New host implementations (Windows, FreeBSD, …) inherit the rule
|
||||
via the interface comment, the `applyStaleClones` helper, and this
|
||||
ADR. The race detector catches violations on normal test runs
|
||||
once `go test -race` is part of CI.
|
||||
- The contract does not cover external state hidden behind the
|
||||
Export (the IOKit handle, the kernel binding). Hosts continue to
|
||||
manage that under their own locks; the rule is specifically about
|
||||
fields read through the `Export` interface methods.
|
||||
|
||||
## References
|
||||
|
||||
- `service/usbip/host.go` — interface doc and `applyStaleClones`.
|
||||
- `service/usbip/host_linux.go` — `cloneLinuxExport`, `committed`
|
||||
map pattern in `Reconcile`.
|
||||
- `service/usbip/host_darwin.go` — `cloneDarwinExport`, mirrored
|
||||
`Reconcile` after this ADR.
|
||||
@@ -13,11 +13,6 @@ import (
|
||||
"golang.org/x/sys/unix"
|
||||
)
|
||||
|
||||
var (
|
||||
errResponseNegativeActualLength = E.New("RET_SUBMIT actual_length is negative")
|
||||
errResponseOverflow = E.New("RET_SUBMIT actual_length exceeds request length")
|
||||
)
|
||||
|
||||
type darwinEndpoint struct {
|
||||
ctx context.Context
|
||||
cancel context.CancelFunc
|
||||
@@ -208,21 +203,19 @@ func (e *darwinEndpoint) finalizePending(pending *pendingTransfer) {
|
||||
}
|
||||
|
||||
// validateResponse reconciles a RET_SUBMIT against the original request shape.
|
||||
// Every wire-shape rule lives here so accept can assume well-formed input and
|
||||
// future defects land in one place. Returns the wire-level errno status and a
|
||||
// non-nil err on protocol violation.
|
||||
// All wire-shape rules live here so accept can assume well-formed input.
|
||||
func (p *pendingTransfer) validateResponse(response SubmitResponse) (int32, error) {
|
||||
if response.ActualLength < 0 {
|
||||
return -int32(unix.EPROTO), errResponseNegativeActualLength
|
||||
return -int32(unix.EPROTO), E.New("RET_SUBMIT actual_length is negative: ", response.ActualLength)
|
||||
}
|
||||
if int(response.ActualLength) > p.requestLen {
|
||||
return -int32(unix.EOVERFLOW), errResponseOverflow
|
||||
return -int32(unix.EOVERFLOW), E.New("RET_SUBMIT actual_length exceeds request length: actual_length ", response.ActualLength, ", request ", p.requestLen)
|
||||
}
|
||||
if p.direction != USBIPDirIn {
|
||||
return 0, nil
|
||||
}
|
||||
if len(response.Buffer) > p.requestLen {
|
||||
return -int32(unix.EOVERFLOW), errResponseOverflow
|
||||
return -int32(unix.EOVERFLOW), E.New("RET_SUBMIT buffer exceeds request length: buffer ", len(response.Buffer), ", request ", p.requestLen)
|
||||
}
|
||||
if len(response.IsoPackets) > 0 {
|
||||
err := ValidateIsoResponse(p.requestLen, int(response.ActualLength), response.IsoPackets, len(response.Buffer))
|
||||
|
||||
@@ -13,34 +13,25 @@ import (
|
||||
"github.com/sagernet/sing-box/log"
|
||||
)
|
||||
|
||||
// exportLedger uses two internal mutexes that are NEVER held
|
||||
// simultaneously. Each public method acquires at most one at a time;
|
||||
// multi-stage methods acquire one, do unlocked work (including
|
||||
// syscalls), then acquire the other.
|
||||
//
|
||||
// fast: broadcast bookkeeping. seq, nextSubID, subs, state.
|
||||
// inventory: exports, busy, leases, nextLeaseID. Direct access is
|
||||
// forbidden; route every read/write through one of
|
||||
// withInventoryRead / withInventoryWrite / withInventoryWriteQuiet
|
||||
// so the broadcast-on-mutation invariant cannot be skipped by
|
||||
// a future caller. The field is unexported within the package
|
||||
// to make any l.inventory.Lock() bypass obvious in review.
|
||||
// exportLedger holds two mutexes that are never acquired together. The
|
||||
// inventory lock must be released before BroadcastIfChanged re-takes it
|
||||
// through snapshotDeviceState.
|
||||
type exportLedger struct {
|
||||
logger log.ContextLogger
|
||||
now func() time.Time
|
||||
ttl time.Duration
|
||||
|
||||
fast sync.Mutex
|
||||
seq uint64
|
||||
nextSubID uint64
|
||||
subs map[uint64]*exportSubscriber
|
||||
state map[string]DeviceInfoV2
|
||||
broadcastAccess sync.Mutex
|
||||
seq uint64
|
||||
nextSubID uint64
|
||||
subs map[uint64]*exportSubscriber
|
||||
state map[string]DeviceInfoV2
|
||||
|
||||
inventory sync.Mutex
|
||||
exports map[string]Export
|
||||
busy map[string]bool
|
||||
leases map[string]serverImportLease
|
||||
nextLeaseID uint64
|
||||
inventoryAccess sync.Mutex
|
||||
exports map[string]Export
|
||||
busy map[string]bool
|
||||
leases map[string]serverImportLease
|
||||
nextLeaseID uint64
|
||||
}
|
||||
|
||||
type exportSubscriber struct {
|
||||
@@ -81,43 +72,28 @@ func newExportLedger(logger log.ContextLogger, ttl time.Duration, now func() tim
|
||||
}
|
||||
}
|
||||
|
||||
// withInventoryWrite holds the inventory lock for body, then broadcasts
|
||||
// iff body reports a change. This is the only path that may both mutate
|
||||
// reserved state AND fire the resulting broadcast; routing every
|
||||
// mutation through here makes the "any reservedLocked-affecting change
|
||||
// must broadcast" invariant structural rather than a discipline rule.
|
||||
// body must NOT acquire fast (lock-ordering rule documented on
|
||||
// exportLedger).
|
||||
// withInventoryWrite broadcasts iff body returns true. body must not
|
||||
// acquire the broadcast lock.
|
||||
func (l *exportLedger) withInventoryWrite(ctx context.Context, body func() bool) {
|
||||
l.inventory.Lock()
|
||||
l.inventoryAccess.Lock()
|
||||
changed := body()
|
||||
l.inventory.Unlock()
|
||||
l.inventoryAccess.Unlock()
|
||||
if changed {
|
||||
l.BroadcastIfChanged(ctx)
|
||||
}
|
||||
}
|
||||
|
||||
// withInventoryRead holds the inventory lock for body without
|
||||
// broadcasting. Use for read-only sections that observe reserved state.
|
||||
func (l *exportLedger) withInventoryRead(body func()) {
|
||||
l.inventory.Lock()
|
||||
defer l.inventory.Unlock()
|
||||
l.inventoryAccess.Lock()
|
||||
defer l.inventoryAccess.Unlock()
|
||||
body()
|
||||
}
|
||||
|
||||
// withInventoryWriteQuiet holds the inventory lock for body without
|
||||
// broadcasting. Use ONLY when the caller is contractually responsible
|
||||
// for the broadcast at a different point in its flow:
|
||||
//
|
||||
// - ApplyHostSnapshot: paired with reconcileAndBroadcast in server.go.
|
||||
// - TryReserveForImport stage 2: the server's import setup broadcasts
|
||||
// after the full session is wired up.
|
||||
// - ResetForClose: shutdown path; subscribers are about to be closed.
|
||||
//
|
||||
// Every other write site MUST use withInventoryWrite.
|
||||
// withInventoryWriteQuiet is for mutations whose broadcast is the
|
||||
// caller's responsibility (paired with BroadcastIfChanged or shutdown).
|
||||
func (l *exportLedger) withInventoryWriteQuiet(body func()) {
|
||||
l.inventory.Lock()
|
||||
defer l.inventory.Unlock()
|
||||
l.inventoryAccess.Lock()
|
||||
defer l.inventoryAccess.Unlock()
|
||||
body()
|
||||
}
|
||||
|
||||
@@ -129,9 +105,7 @@ func (l *exportLedger) IsReserved(busid string) bool {
|
||||
return reserved
|
||||
}
|
||||
|
||||
// reservedLocked reports whether busid is unavailable for new admission:
|
||||
// either marked busy by an active session or covered by an unexpired
|
||||
// import lease. Caller must hold l.inventory.
|
||||
// reservedLocked: caller must hold l.inventoryAccess.
|
||||
func (l *exportLedger) reservedLocked(busid string) bool {
|
||||
if l.busy[busid] {
|
||||
return true
|
||||
@@ -173,20 +147,20 @@ func (l *exportLedger) ApplyHostSnapshot(snapshot map[string]Export, released []
|
||||
|
||||
func (l *exportLedger) SeedBroadcastState(ctx context.Context) {
|
||||
nextState := deviceInfoV2Map(l.snapshotDeviceState(ctx))
|
||||
l.fast.Lock()
|
||||
l.broadcastAccess.Lock()
|
||||
l.state = nextState
|
||||
l.fast.Unlock()
|
||||
l.broadcastAccess.Unlock()
|
||||
}
|
||||
|
||||
func (l *exportLedger) BroadcastIfChanged(ctx context.Context) bool {
|
||||
nextState := deviceInfoV2Map(l.snapshotDeviceState(ctx))
|
||||
|
||||
l.fast.Lock()
|
||||
l.broadcastAccess.Lock()
|
||||
nextSequence := l.seq + 1
|
||||
delta := buildControlDeviceDelta(nextSequence, l.state, nextState)
|
||||
if len(delta.Added) == 0 && len(delta.Updated) == 0 && len(delta.Removed) == 0 {
|
||||
l.state = nextState
|
||||
l.fast.Unlock()
|
||||
l.broadcastAccess.Unlock()
|
||||
return false
|
||||
}
|
||||
l.seq = nextSequence
|
||||
@@ -196,7 +170,7 @@ func (l *exportLedger) BroadcastIfChanged(ctx context.Context) bool {
|
||||
for _, sub := range l.subs {
|
||||
targets = append(targets, sub)
|
||||
}
|
||||
l.fast.Unlock()
|
||||
l.broadcastAccess.Unlock()
|
||||
|
||||
frame := controlFrame{
|
||||
Type: controlFrameChanged,
|
||||
@@ -217,13 +191,9 @@ func (l *exportLedger) BroadcastIfChanged(ctx context.Context) bool {
|
||||
return true
|
||||
}
|
||||
|
||||
// TryReserveForImport runs Export.LeaseCheck outside the inventory lock;
|
||||
// the busy mark is inserted only after a second availability re-check
|
||||
// confirms no goroutine raced in. An outstanding lease counts as busy so
|
||||
// legacy OP_REQ_IMPORT cannot steal a slot a control client has already
|
||||
// reserved via IssueLease. The caller must pair every success with a
|
||||
// later ReleaseImport, and is responsible for broadcasting the busy
|
||||
// transition once the session is fully wired up.
|
||||
// TryReserveForImport runs LeaseCheck outside the lock and re-checks
|
||||
// availability before marking busy. Caller must pair success with
|
||||
// ReleaseImport and broadcast once the session is wired up.
|
||||
func (l *exportLedger) TryReserveForImport(ctx context.Context, busid string) (Export, bool, string) {
|
||||
var (
|
||||
export Export
|
||||
@@ -278,13 +248,8 @@ func (l *exportLedger) ReleaseImport(ctx context.Context, busid string, removeEx
|
||||
})
|
||||
}
|
||||
|
||||
// IssueLease captures the current broadcast sequence as opaque metadata
|
||||
// for control clients. Lease correctness itself is pinned to the
|
||||
// export's internal identity, not to that sequence number. Both
|
||||
// inventory stages run through withInventoryWrite so any lease insert,
|
||||
// re-validation, or TTL sweep broadcasts the resulting reserved-state
|
||||
// change to subscribers — without this, an unexpired lease would block
|
||||
// new imports while extended clients still saw the device as available.
|
||||
// IssueLease pins lease correctness to the export identity; the
|
||||
// broadcast sequence on the response is opaque metadata for clients.
|
||||
func (l *exportLedger) IssueLease(ctx context.Context, subID uint64, request controlLeaseRequest) controlLeaseResponse {
|
||||
response := controlLeaseResponse{
|
||||
BusID: request.BusID,
|
||||
@@ -296,9 +261,9 @@ func (l *exportLedger) IssueLease(ctx context.Context, subID uint64, request con
|
||||
return response
|
||||
}
|
||||
|
||||
l.fast.Lock()
|
||||
l.broadcastAccess.Lock()
|
||||
generation := l.seq
|
||||
l.fast.Unlock()
|
||||
l.broadcastAccess.Unlock()
|
||||
|
||||
var (
|
||||
export Export
|
||||
@@ -378,16 +343,9 @@ func (l *exportLedger) IssueLease(ctx context.Context, subID uint64, request con
|
||||
return response
|
||||
}
|
||||
|
||||
// ConsumeLeaseAndReserve validates the requested lease against the
|
||||
// current export identity, reruns LeaseCheck outside the inventory lock,
|
||||
// then atomically consumes the lease and marks the busid busy.
|
||||
// Correctness is tied to the export identity rather than to the control
|
||||
// sequence.
|
||||
//
|
||||
// Consume-on-read semantics from the old ConsumeLease are preserved:
|
||||
// the lease entry is removed on every outcome except a nonce/ID
|
||||
// mismatch (which preserves the lease for the legitimate holder).
|
||||
// The caller must pair every success with a later ReleaseImport.
|
||||
// ConsumeLeaseAndReserve consumes the lease on every outcome except an
|
||||
// ID/nonce mismatch (the latter preserves the lease for the legitimate
|
||||
// holder). Caller must pair success with ReleaseImport.
|
||||
func (l *exportLedger) ConsumeLeaseAndReserve(ctx context.Context, request ImportExtRequest) (Export, bool, string) {
|
||||
var (
|
||||
export Export
|
||||
@@ -527,23 +485,23 @@ func (l *exportLedger) Subscribe(ctx context.Context, conn net.Conn, capabilitie
|
||||
if extended {
|
||||
// Keep the snapshot and sequence from the same stable generation.
|
||||
for {
|
||||
l.fast.Lock()
|
||||
l.broadcastAccess.Lock()
|
||||
sequence = l.seq
|
||||
l.fast.Unlock()
|
||||
l.broadcastAccess.Unlock()
|
||||
|
||||
snapshot = l.snapshotDeviceState(ctx)
|
||||
|
||||
l.fast.Lock()
|
||||
l.broadcastAccess.Lock()
|
||||
if sequence == l.seq {
|
||||
break
|
||||
}
|
||||
l.fast.Unlock()
|
||||
l.broadcastAccess.Unlock()
|
||||
}
|
||||
} else {
|
||||
l.fast.Lock()
|
||||
l.broadcastAccess.Lock()
|
||||
sequence = l.seq
|
||||
}
|
||||
defer l.fast.Unlock()
|
||||
defer l.broadcastAccess.Unlock()
|
||||
l.nextSubID++
|
||||
sub := &exportSubscriber{
|
||||
id: l.nextSubID,
|
||||
@@ -572,9 +530,9 @@ func (l *exportLedger) Subscribe(ctx context.Context, conn net.Conn, capabilitie
|
||||
// broadcast so remaining subscribers see the busid become available
|
||||
// again.
|
||||
func (l *exportLedger) Unsubscribe(ctx context.Context, sub *exportSubscriber) {
|
||||
l.fast.Lock()
|
||||
l.broadcastAccess.Lock()
|
||||
delete(l.subs, sub.id)
|
||||
l.fast.Unlock()
|
||||
l.broadcastAccess.Unlock()
|
||||
l.withInventoryWrite(ctx, func() bool {
|
||||
released := false
|
||||
for busid, lease := range l.leases {
|
||||
@@ -590,13 +548,13 @@ func (l *exportLedger) Unsubscribe(ctx context.Context, sub *exportSubscriber) {
|
||||
// CloseAllSubscribers returns the underlying connections so the caller
|
||||
// can close them outside any lock.
|
||||
func (l *exportLedger) CloseAllSubscribers() []net.Conn {
|
||||
l.fast.Lock()
|
||||
l.broadcastAccess.Lock()
|
||||
conns := make([]net.Conn, 0, len(l.subs))
|
||||
for _, sub := range l.subs {
|
||||
conns = append(conns, sub.conn)
|
||||
}
|
||||
l.subs = make(map[uint64]*exportSubscriber)
|
||||
l.fast.Unlock()
|
||||
l.broadcastAccess.Unlock()
|
||||
return conns
|
||||
}
|
||||
|
||||
@@ -612,9 +570,9 @@ func (l *exportLedger) HandleControlLeaseRequest(ctx context.Context, sub *expor
|
||||
var request controlLeaseRequest
|
||||
err := unmarshalControlPayload(payload, &request)
|
||||
if err != nil {
|
||||
l.fast.Lock()
|
||||
l.broadcastAccess.Lock()
|
||||
sequence := l.seq
|
||||
l.fast.Unlock()
|
||||
l.broadcastAccess.Unlock()
|
||||
l.enqueuePayload(sub, controlFrame{
|
||||
Type: controlFrameLeaseResponse,
|
||||
Version: controlProtocolVersion,
|
||||
@@ -625,9 +583,9 @@ func (l *exportLedger) HandleControlLeaseRequest(ctx context.Context, sub *expor
|
||||
return
|
||||
}
|
||||
response := l.IssueLease(ctx, sub.id, request)
|
||||
l.fast.Lock()
|
||||
l.broadcastAccess.Lock()
|
||||
sequence := l.seq
|
||||
l.fast.Unlock()
|
||||
l.broadcastAccess.Unlock()
|
||||
l.enqueuePayload(sub, controlFrame{
|
||||
Type: controlFrameLeaseResponse,
|
||||
Version: controlProtocolVersion,
|
||||
|
||||
+13
-60
@@ -7,22 +7,15 @@ import (
|
||||
"net"
|
||||
)
|
||||
|
||||
// ExportHost lifecycle: Start → Reconcile* → Close. Reconcile may be
|
||||
// called many times; it returns the committed post-reconcile export
|
||||
// state, and callers must apply snapshot/released even when err != nil.
|
||||
// FinishImport runs after each data session ends so the platform can do
|
||||
// post-import cleanup (Linux: write -1 to usbip_sockfd; Darwin:
|
||||
// release stale-marked captures).
|
||||
// ExportHost lifecycle: Start → Reconcile* → Close. Callers must apply
|
||||
// the Reconcile snapshot and released list even when err != nil.
|
||||
type ExportHost interface {
|
||||
Start(ctx context.Context) error
|
||||
Close() error
|
||||
Reconcile(ctx context.Context, isReserved func(busid string) bool) (snapshot map[string]Export, released []string, err error)
|
||||
FinishImport(ctx context.Context, busid string) (released bool, err error)
|
||||
// Events returns a coalescing channel that signals topology
|
||||
// changes. The channel is closed when Close() is called, which
|
||||
// also stops the host's background goroutines. Events MUST be
|
||||
// called after Start succeeds. A non-nil error means the host
|
||||
// could not subscribe and the server must not continue.
|
||||
// Events MUST be called after Start succeeds. The channel closes when
|
||||
// Close() runs.
|
||||
Events() (<-chan struct{}, error)
|
||||
}
|
||||
|
||||
@@ -35,20 +28,10 @@ type ImportHost interface {
|
||||
|
||||
type ExportLeaseIdentity string
|
||||
|
||||
// Export is the host-owned handle to a single USB device that the ledger
|
||||
// publishes to control subscribers and hands to import sessions.
|
||||
//
|
||||
// Pointers returned from ExportHost.Reconcile MUST be treated as
|
||||
// immutable from the ledger's perspective: every field read by
|
||||
// BusID, Snapshot, LeaseIdentity, LeaseCheck, or DeviceInfo must not
|
||||
// change after the Export is published into the snapshot map. The
|
||||
// ledger calls these methods outside any lock — concurrent mutation is
|
||||
// a data race. Hosts that need to change such state (mark stale, swap
|
||||
// underlying device handle, …) MUST publish a fresh Export pointer by
|
||||
// cloning, mutating the clone, then swapping it into the host's
|
||||
// committed map under the host's own lock. See cloneLinuxExport and
|
||||
// docs/adr/0001-export-pointer-immutability.md for the canonical
|
||||
// pattern; applyStaleClones enforces the rule for stale transitions.
|
||||
// Export pointers handed back from Reconcile are immutable from the
|
||||
// ledger's perspective: the ledger calls the methods below outside any
|
||||
// lock. Hosts that need to mutate must clone, mutate the clone, then
|
||||
// swap it into their committed map under the host's own lock.
|
||||
type Export interface {
|
||||
BusID() string
|
||||
Snapshot(ctx context.Context, busy bool) ExportSnapshot
|
||||
@@ -58,31 +41,8 @@ type Export interface {
|
||||
NewServerDataSession(ctx context.Context, conn net.Conn) (DataSession, error)
|
||||
}
|
||||
|
||||
// applyStaleClones honours the Export immutability contract for
|
||||
// stale-mark transitions: for every busid in toStale that exists in
|
||||
// committed, it allocates a clone, runs mark on the clone, and writes
|
||||
// the clone back. The previously published pointer is never mutated.
|
||||
// Callers that consumed the original pointer continue to observe its
|
||||
// pre-stale snapshot until they next refresh from committed.
|
||||
func applyStaleClones[T any](committed map[string]*T, toStale []string, clone func(*T) *T, mark func(*T)) {
|
||||
for _, busid := range toStale {
|
||||
exp, found := committed[busid]
|
||||
if !found {
|
||||
continue
|
||||
}
|
||||
cloned := clone(exp)
|
||||
mark(cloned)
|
||||
committed[busid] = cloned
|
||||
}
|
||||
}
|
||||
|
||||
// ExportSnapshot Backend and StableID fields are populated
|
||||
// unconditionally. Unavailable snapshots keep a cached Entry,
|
||||
// including BusID, so the caller can broadcast a state transition
|
||||
// instead of removing the device outright; snapshots without a BusID
|
||||
// are treated as non-broadcastable. ExportHost.Reconcile returns
|
||||
// stale entries in its snapshot for the same reason — the ledger
|
||||
// filters non-broadcastable snapshots in one place, not the host.
|
||||
// ExportSnapshot entries with an empty BusID are filtered out by the
|
||||
// ledger; hosts may keep stale snapshots so state transitions broadcast.
|
||||
type ExportSnapshot struct {
|
||||
Entry DeviceEntry
|
||||
Backend string
|
||||
@@ -92,16 +52,9 @@ type ExportSnapshot struct {
|
||||
RawStatus int
|
||||
}
|
||||
|
||||
// DataSession implementations MUST close the channel returned by Done
|
||||
// when the session terminates for any reason. Err is only valid after
|
||||
// Done is closed; it returns nil for a clean detach. Close is idempotent
|
||||
// and safe to call from any goroutine.
|
||||
//
|
||||
// Start transfers the session from "prepared" to "running". The session
|
||||
// is returned in the prepared state with the userspace conn still alive
|
||||
// so the server can send OP_REP_IMPORT before the kernel takes wire
|
||||
// ownership. Start is idempotent. Close before Start releases all
|
||||
// resources, including the userspace conn.
|
||||
// DataSession implementations MUST close Done when the session
|
||||
// terminates. Err is only valid after Done. Start and Close are
|
||||
// idempotent; Close before Start releases all resources.
|
||||
type DataSession interface {
|
||||
Done() <-chan struct{}
|
||||
Err() error
|
||||
|
||||
@@ -178,25 +178,20 @@ func (h *darwinExportHost) Reconcile(ctx context.Context, isReserved func(busid
|
||||
released = append(released, busid)
|
||||
}
|
||||
|
||||
// Build the committed map off-lock so we can clone any stale entry
|
||||
// before mutating it. The ledger reads Snapshot/LeaseCheck on the
|
||||
// previously published pointers without locks; mutating them in
|
||||
// place is a data race. See docs/adr/0001-export-pointer-immutability.md.
|
||||
committed := make(map[string]*darwinExport, len(current)+len(toAdd))
|
||||
maps.Copy(committed, current)
|
||||
staleBusIDs := make([]string, 0, len(toStale))
|
||||
pendingByBusID := make(map[string]uint64, len(toStale))
|
||||
for _, mark := range toStale {
|
||||
staleBusIDs = append(staleBusIDs, mark.busid)
|
||||
pendingByBusID[mark.busid] = mark.pendingRegistryID
|
||||
}
|
||||
applyStaleClones(committed, staleBusIDs, cloneDarwinExport, func(exp *darwinExport) {
|
||||
exp.stale = true
|
||||
pending := pendingByBusID[exp.busid]
|
||||
if pending != 0 {
|
||||
exp.pendingRegistryID = pending
|
||||
exp, found := committed[mark.busid]
|
||||
if !found {
|
||||
continue
|
||||
}
|
||||
})
|
||||
cloned := cloneDarwinExport(exp)
|
||||
cloned.stale = true
|
||||
if mark.pendingRegistryID != 0 {
|
||||
cloned.pendingRegistryID = mark.pendingRegistryID
|
||||
}
|
||||
committed[mark.busid] = cloned
|
||||
}
|
||||
for _, exp := range toRemove {
|
||||
delete(committed, exp.busid)
|
||||
}
|
||||
@@ -281,11 +276,6 @@ func snapshotDarwinExports(exports map[string]*darwinExport) map[string]Export {
|
||||
return out
|
||||
}
|
||||
|
||||
// cloneDarwinExport returns a fresh *darwinExport with the slice fields
|
||||
// inside entry duplicated so callers can mutate the clone without
|
||||
// affecting the previously published pointer. The device handle and
|
||||
// logger are shared because they are externally synchronised. See
|
||||
// docs/adr/0001-export-pointer-immutability.md.
|
||||
func cloneDarwinExport(exp *darwinExport) *darwinExport {
|
||||
if exp == nil {
|
||||
return nil
|
||||
|
||||
@@ -340,9 +340,15 @@ func (h *linuxExportHost) Reconcile(ctx context.Context, isReserved func(busid s
|
||||
maps.Copy(committed, current)
|
||||
var reconcileErrors []error
|
||||
|
||||
applyStaleClones(committed, plan.toStale, cloneLinuxExport, func(exp *linuxExport) {
|
||||
exp.stale = true
|
||||
})
|
||||
for _, busid := range plan.toStale {
|
||||
exp, found := committed[busid]
|
||||
if !found {
|
||||
continue
|
||||
}
|
||||
cloned := cloneLinuxExport(exp)
|
||||
cloned.stale = true
|
||||
committed[busid] = cloned
|
||||
}
|
||||
|
||||
for _, exp := range plan.toRelease {
|
||||
releaseErr := h.releaseExport(exp)
|
||||
|
||||
@@ -35,36 +35,26 @@ func RebaseFrame(currentFrame uint64, low8 uint8) uint64 {
|
||||
return base
|
||||
}
|
||||
|
||||
var (
|
||||
errIsoDescriptorCount = E.New("RET_SUBMIT iso descriptor count mismatch")
|
||||
errIsoDescriptorRange = E.New("RET_SUBMIT iso descriptor range mismatch")
|
||||
errIsoDescriptorActualLength = E.New("RET_SUBMIT iso descriptor actual_length exceeds length")
|
||||
errIsoDescriptorSum = E.New("RET_SUBMIT iso descriptor actual_length sum does not match header")
|
||||
errIsoPayloadShort = E.New("RET_SUBMIT iso payload shorter than descriptor range")
|
||||
)
|
||||
|
||||
// ValidateIsoResponse enforces the per-descriptor invariants of a RET_SUBMIT
|
||||
// against the original CMD_SUBMIT shape. startIsoTransfer emits single-packet
|
||||
// CMD_SUBMITs that cover the whole request, so the response must mirror that
|
||||
// exact shape. If multi-packet ISO submits are added later, the count check
|
||||
// relaxes to a non-empty count and the offset/length check becomes a per-
|
||||
// descriptor walk that proves coverage is non-overlapping and in-range.
|
||||
// exact shape.
|
||||
func ValidateIsoResponse(requestLen int, actualLength int, packets []IsoPacketDescriptor, payloadLen int) error {
|
||||
if len(packets) != 1 {
|
||||
return E.Extend(errIsoDescriptorCount, "expected 1, got ", len(packets))
|
||||
return E.New("RET_SUBMIT iso descriptor count mismatch: expected 1, got ", len(packets))
|
||||
}
|
||||
descriptor := packets[0]
|
||||
if descriptor.Offset != 0 || int(descriptor.Length) != requestLen {
|
||||
return E.Extend(errIsoDescriptorRange, "offset ", descriptor.Offset, ", length ", descriptor.Length, ", request ", requestLen)
|
||||
return E.New("RET_SUBMIT iso descriptor range mismatch: offset ", descriptor.Offset, ", length ", descriptor.Length, ", request ", requestLen)
|
||||
}
|
||||
if descriptor.ActualLength < 0 || descriptor.ActualLength > descriptor.Length {
|
||||
return E.Extend(errIsoDescriptorActualLength, "actual_length ", descriptor.ActualLength, ", length ", descriptor.Length)
|
||||
return E.New("RET_SUBMIT iso descriptor actual_length exceeds length: actual_length ", descriptor.ActualLength, ", length ", descriptor.Length)
|
||||
}
|
||||
if int(descriptor.ActualLength) != actualLength {
|
||||
return E.Extend(errIsoDescriptorSum, "sum ", descriptor.ActualLength, " != header ", actualLength)
|
||||
return E.New("RET_SUBMIT iso descriptor actual_length sum does not match header: sum ", descriptor.ActualLength, " != header ", actualLength)
|
||||
}
|
||||
if int(descriptor.ActualLength) > payloadLen {
|
||||
return E.Extend(errIsoPayloadShort, "actual_length ", descriptor.ActualLength, " > payload ", payloadLen)
|
||||
return E.New("RET_SUBMIT iso payload shorter than descriptor range: actual_length ", descriptor.ActualLength, " > payload ", payloadLen)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -75,13 +75,8 @@ type darwinCITransfer struct {
|
||||
message darwinCIMessage
|
||||
}
|
||||
|
||||
// cgoCallbackHandle pairs a cgo.Handle whose value is invoked asynchronously
|
||||
// from C with the ordering rule that the handle MUST NOT be deleted until the
|
||||
// C-side producer has been destroyed AND any in-flight callbacks have been
|
||||
// drained. closeAfter encodes that ordering: the destroyC callback runs first,
|
||||
// and only on its return is the handle deleted. destroyC MUST NOT return
|
||||
// until the C-side serial dispatch queue has been synchronously drained (see
|
||||
// box_usbhost_drain_and_release_queue in usbhost_darwin.m).
|
||||
// cgoCallbackHandle: destroyC MUST synchronously drain the C-side queue
|
||||
// before returning; only then is it safe to delete the handle.
|
||||
type cgoCallbackHandle struct {
|
||||
handle cgo.Handle
|
||||
}
|
||||
@@ -94,8 +89,7 @@ func (c cgoCallbackHandle) token() C.uintptr_t {
|
||||
return C.uintptr_t(c.handle)
|
||||
}
|
||||
|
||||
// deleteRaw releases the handle without waiting for any C-side drain. Use only
|
||||
// to roll back a failed C-side create; otherwise prefer closeAfter.
|
||||
// deleteRaw rolls back a failed C-side create without a drain.
|
||||
func (c cgoCallbackHandle) deleteRaw() {
|
||||
c.handle.Delete()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user