From 05ac804e6c0d758aac36fbe6657dce09a89201bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=B8=96=E7=95=8C?= Date: Wed, 10 Jun 2026 09:23:29 +0800 Subject: [PATCH] usbip: make export ledger broadcasts atomic BroadcastIfChanged computed the snapshot under inventoryAccess and published it under broadcastAccess as two separate steps, so two racing broadcasts could publish in the opposite order they computed, locking a stale snapshot in as the authoritative state seeded to new subscribers. Compute, publish, and subscriber enqueue now happen in one broadcastAccess critical section (enqueue is a non-blocking send). --- service/usbip/export_ledger.go | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/service/usbip/export_ledger.go b/service/usbip/export_ledger.go index 61d792ab2..e77fcf829 100644 --- a/service/usbip/export_ledger.go +++ b/service/usbip/export_ledger.go @@ -13,10 +13,12 @@ import ( "github.com/sagernet/sing-box/log" ) -// exportLedger holds two mutexes that are never acquired together. -// BroadcastIfChanged computes the next state under inventoryAccess via -// snapshotDeviceState, then takes broadcastAccess to compare against -// l.state, swap it in, and snapshot the subscriber list. +// exportLedger lock order: broadcastAccess may be taken first and then +// inventoryAccess nested inside (BroadcastIfChanged computes the next +// state via snapshotDeviceState while holding broadcastAccess, so that +// compute and publish are one atomic step — two racing broadcasts can +// otherwise publish out of order and lock in a stale snapshot). Code +// holding inventoryAccess must never take broadcastAccess. type exportLedger struct { logger log.ContextLogger now func() time.Time @@ -113,31 +115,27 @@ func (l *exportLedger) ApplyHostSnapshot(snapshot map[string]Export, released [] } func (l *exportLedger) SeedBroadcastState() { - nextState := controlDeviceInfoMap(l.snapshotDeviceState()) l.broadcastAccess.Lock() - l.state = nextState + l.state = controlDeviceInfoMap(l.snapshotDeviceState()) l.broadcastAccess.Unlock() } +// BroadcastIfChanged computes, publishes, and enqueues the snapshot in +// one broadcastAccess critical section. Enqueueing outside the lock +// would let two racing broadcasts deliver in the opposite order they +// published, leaving subscribers on the stale snapshot. Enqueue is a +// non-blocking channel send, so holding the lock across it is cheap. func (l *exportLedger) BroadcastIfChanged() bool { - nextState := controlDeviceInfoMap(l.snapshotDeviceState()) - l.broadcastAccess.Lock() + defer l.broadcastAccess.Unlock() + nextState := controlDeviceInfoMap(l.snapshotDeviceState()) if maps.EqualFunc(l.state, nextState, controlDeviceInfoEqual) { - l.broadcastAccess.Unlock() return false } l.state = nextState - devices := sortedControlDeviceInfoValues(nextState) - targets := make([]*exportSubscriber, 0, len(l.subs)) - for _, sub := range l.subs { - targets = append(targets, sub) - } - l.broadcastAccess.Unlock() - frame := controlFrame{Type: controlFrameDeviceSnapshot, Version: controlProtocolVersion} - payload := controlDeviceSnapshot{Devices: devices} - for _, sub := range targets { + payload := controlDeviceSnapshot{Devices: sortedControlDeviceInfoValues(nextState)} + for _, sub := range l.subs { l.enqueuePayload(sub, frame, payload) } return true