Serialize Darwin USB host teardown

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:

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()