From f1a516cb6e307cc6d750492a272364e49033c14c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=B8=96=E7=95=8C?= Date: Fri, 24 Apr 2026 07:55:54 +0800 Subject: [PATCH] Serialize Darwin USB host teardown --- service/usbip/client_darwin.go | 75 ++++++++++++++----- service/usbip/darwin_integration_test.go | 95 +++++++++++++++++++++++- 2 files changed, 147 insertions(+), 23 deletions(-) diff --git a/service/usbip/client_darwin.go b/service/usbip/client_darwin.go index 209943b13..7b3bcfdd7 100644 --- a/service/usbip/client_darwin.go +++ b/service/usbip/client_darwin.go @@ -682,11 +682,13 @@ type darwinVirtualController struct { info DeviceInfoTruncated startTime time.Time - controller *darwinUSBHostController - events chan darwinControllerEvent - done chan struct{} - closeOnce sync.Once - seq atomic.Uint32 + controller *darwinUSBHostController + events chan darwinControllerEvent + done chan struct{} + eventDone chan struct{} + closeOnce sync.Once + eventStarted atomic.Bool + seq atomic.Uint32 writeMu sync.Mutex pendingMu sync.Mutex @@ -712,6 +714,7 @@ func newDarwinVirtualController(ctx context.Context, logger log.ContextLogger, c startTime: time.Now(), events: make(chan darwinControllerEvent, 64), done: make(chan struct{}), + eventDone: make(chan struct{}), pending: make(map[uint32]darwinPendingSubmit), nextAddress: 1, devices: make(map[uint8]*darwinUSBHostDeviceSM), @@ -726,26 +729,25 @@ func (c *darwinVirtualController) Start() error { return err } c.controller = controller + c.eventStarted.Store(true) go c.readLoop() go c.eventLoop() return nil } func (c *darwinVirtualController) Close() { + c.requestClose() + if c.eventStarted.Load() { + <-c.eventDone + } +} + +func (c *darwinVirtualController) requestClose() { c.closeOnce.Do(func() { c.cancel() - _ = c.conn.Close() - if c.controller != nil { - c.controller.Close() + if c.conn != nil { + _ = c.conn.Close() } - c.stateMu.Lock() - for _, endpoint := range c.endpoints { - endpoint.Close() - } - for _, device := range c.devices { - device.Close() - } - c.stateMu.Unlock() }) } @@ -767,7 +769,7 @@ func (c *darwinVirtualController) enqueueEvent(event darwinControllerEvent) { case <-c.ctx.Done(): default: c.logger.Warn("IOUSBHostControllerInterface event queue overflow") - c.Close() + c.requestClose() } } @@ -812,6 +814,9 @@ func (c *darwinVirtualController) readLoop() { } func (c *darwinVirtualController) eventLoop() { + c.eventStarted.Store(true) + defer close(c.eventDone) + defer c.teardownIOUSBHostState() for { select { case <-c.ctx.Done(): @@ -822,6 +827,9 @@ func (c *darwinVirtualController) eventLoop() { } else { c.handleDoorbell(event.doorbell) } + if c.ctx.Err() != nil { + return + } } } } @@ -861,7 +869,8 @@ func (c *darwinVirtualController) handleCommand(message darwinCIMessage) { } if err != nil { c.logger.Debug("IOUSBHostCI command 0x", hex8(message.messageType()), ": ", err) - c.Close() + c.requestClose() + return } } @@ -960,12 +969,40 @@ func (c *darwinVirtualController) handleDoorbell(doorbell uint32) { status, length := c.handleTransfer(key, transfer.message) if err := endpoint.complete(transfer, darwinUSBIPStatusToCIStatus(status), length); err != nil { c.logger.Debug("complete transfer: ", err) - c.Close() + c.requestClose() return } } } +func (c *darwinVirtualController) teardownIOUSBHostState() { + c.stateMu.Lock() + endpoints := make([]darwinEndpointStateMachine, 0, len(c.endpoints)) + for _, endpoint := range c.endpoints { + endpoints = append(endpoints, endpoint) + } + c.endpoints = make(map[darwinEndpointKey]darwinEndpointStateMachine) + devices := make([]*darwinUSBHostDeviceSM, 0, len(c.devices)) + for _, device := range c.devices { + devices = append(devices, device) + } + c.devices = make(map[uint8]*darwinUSBHostDeviceSM) + c.controlStates = make(map[uint8]darwinControlState) + controller := c.controller + c.controller = nil + c.stateMu.Unlock() + + for _, endpoint := range endpoints { + endpoint.Close() + } + for _, device := range devices { + device.Close() + } + if controller != nil { + controller.Close() + } +} + func (c *darwinVirtualController) handleTransfer(key darwinEndpointKey, message darwinCIMessage) (int32, int) { switch message.messageType() { case ciMsgSetupTransfer: diff --git a/service/usbip/darwin_integration_test.go b/service/usbip/darwin_integration_test.go index 20de12593..f0b0eedd8 100644 --- a/service/usbip/darwin_integration_test.go +++ b/service/usbip/darwin_integration_test.go @@ -205,18 +205,34 @@ func TestWaitDarwinControllerClosesOnContextCancel(t *testing.T) { } type fakeDarwinEndpointStateMachine struct { - transfers []darwinCITransfer - currentRead int - completeCalled int + transfers []darwinCITransfer + processDoorbellStarted chan struct{} + releaseProcessDoorbell <-chan struct{} + closeCalled chan struct{} + currentRead int + completeCalled int + closeOnce sync.Once } -func (f *fakeDarwinEndpointStateMachine) Close() {} +func (f *fakeDarwinEndpointStateMachine) Close() { + if f.closeCalled != nil { + f.closeOnce.Do(func() { + close(f.closeCalled) + }) + } +} func (f *fakeDarwinEndpointStateMachine) respond(darwinCIMessage, int) error { return nil } func (f *fakeDarwinEndpointStateMachine) processDoorbell(uint32) error { + if f.processDoorbellStarted != nil { + close(f.processDoorbellStarted) + } + if f.releaseProcessDoorbell != nil { + <-f.releaseProcessDoorbell + } return nil } @@ -285,6 +301,77 @@ func TestDarwinHandleDoorbellContinuesAfterNoResponseTransfer(t *testing.T) { require.Equal(t, 2, endpoint.currentRead) } +func TestDarwinControllerCloseWaitsForEventLoopTeardown(t *testing.T) { + t.Parallel() + + controller := newDarwinVirtualController(context.Background(), newTestLogger(), nil, DeviceInfoTruncated{}) + processStarted := make(chan struct{}) + releaseProcess := make(chan struct{}) + endpointClosed := make(chan struct{}) + endpoint := &fakeDarwinEndpointStateMachine{ + processDoorbellStarted: processStarted, + releaseProcessDoorbell: releaseProcess, + closeCalled: endpointClosed, + } + controller.endpoints[darwinEndpointKey{device: 1, endpoint: 2}] = endpoint + + go controller.eventLoop() + controller.enqueueDoorbell((uint32(2) << 8) | 1) + + select { + case <-processStarted: + case <-time.After(time.Second): + t.Fatal("timed out waiting for doorbell processing") + } + + closeDone := make(chan struct{}) + go func() { + controller.Close() + close(closeDone) + }() + + select { + case <-endpointClosed: + t.Fatal("endpoint closed while doorbell processing was active") + case <-time.After(100 * time.Millisecond): + } + select { + case <-closeDone: + t.Fatal("controller Close returned while doorbell processing was active") + case <-time.After(100 * time.Millisecond): + } + + close(releaseProcess) + + select { + case <-endpointClosed: + case <-time.After(time.Second): + t.Fatal("timed out waiting for endpoint close") + } + select { + case <-closeDone: + case <-time.After(time.Second): + t.Fatal("timed out waiting for controller Close") + } +} + +func TestDarwinControllerCloseWithNilConn(t *testing.T) { + t.Parallel() + + controller := newDarwinVirtualController(context.Background(), newTestLogger(), nil, DeviceInfoTruncated{}) + done := make(chan struct{}) + go func() { + controller.Close() + close(done) + }() + + select { + case <-done: + case <-time.After(time.Second): + t.Fatal("timed out waiting for controller Close") + } +} + func startDarwinFakeUSBIPServer(t *testing.T) *darwinFakeUSBIPServer { t.Helper()