Reopen an interface whose device went away

Interfaces are opened once and kept, which is right while a device stays put
and wrong the moment it does not. A device that disappeared even briefly —
re-enumerating, or a phone changing its USB configuration — left the daemon
holding a dead IOKit handle, and every transfer through it returned "no
device" from then on. Restarting the daemon was the only way back; a fresh
one worked immediately, which is how this was noticed against a Pixel.

A transfer that reports the device gone now drops the cached interface, so
the next request opens a new one. Only that status does: kIOReturnNoDevice
and kIOReturnNotResponding both map to it, while a stall or a timeout leaves
the interface alone. Discarding on any failure would throw away a working
interface on every timeout, and a timeout is the ordinary outcome of reading
an endpoint with nothing to say — this project's own concurrency check
produces one deliberately.

The interface is taken out of the table under the lock and released after
it. Letting it deallocate inside would run its deinit, and so close(), and so
IOKit calls, while every other transfer waited on that lock.

Also fix the release checksums, which have now been wrong twice for the same
reason: the glob names the artifacts and the artifacts get renamed. First it
matched "*.macos" while binaries ended in "-macos", so checksums-v0.5.0
covered only the tarballs. Then per-architecture builds arrived ending in
"-arm64" and "-x86_64", which "*-macos" does not match either, so v0.6.0
shipped with no hash for the two binaries most people download. It now
checksums every artifact, which cannot drift out of step with their names,
and fails the release if any expected binary is missing from the result.
Verified against a mock artifact directory: all eight files covered, and the
old glob trips the new guard on exactly the two binaries v0.6.0 omitted.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017rmyikdjWveP99ZUCDLY89

Author: beriberikix

.github/workflows/release.yml

@@ -406,11 +406,31 @@ jobs:
           CHECKSUMS_FILE="checksums-${{ needs.release-validation.outputs.version }}.sha256"
           
           echo "📊 Generating checksums..."
-          # Only checksum files, not directories
-          # The binaries are named usbipd-vX.Y.Z-macos, so they end in "-macos" and the
-          # old "*.macos" glob never matched one. checksums-v0.5.0.sha256 shipped
-          # covering only the tarballs, with no hash for the binary anyone downloads.
-          find . -maxdepth 1 -type f \( -name "*-macos" -o -name "*.tar.gz" \) -exec shasum -a 256 {} \; > "$CHECKSUMS_FILE"
+          # Checksum every artifact, rather than the ones a pattern happens to name.
+          #
+          # This glob has been wrong twice, each time because the artifacts were renamed
+          # and it was not. First it matched "*.macos" while binaries ended in "-macos",
+          # so checksums-v0.5.0.sha256 covered only the tarballs. Then per-architecture
+          # builds arrived ending in "-arm64" and "-x86_64", which "*-macos" does not
+          # match either, so v0.6.0 shipped without a hash for the two binaries most
+          # people download. Listing everything cannot drift out of step with the names.
+          find . -maxdepth 1 -type f ! -name "$CHECKSUMS_FILE" -exec shasum -a 256 {} \; > "$CHECKSUMS_FILE"
+
+          # And check the binaries by name, so a missing artifact fails here rather than
+          # at somebody's `shasum -c`.
+          MISSING=""
+          for EXPECTED in \
+            "usbipd-${{ needs.release-validation.outputs.version }}-macos" \
+            "usbipd-${{ needs.release-validation.outputs.version }}-macos-arm64" \
+            "usbipd-${{ needs.release-validation.outputs.version }}-macos-x86_64"; do
+            if [ -f "$EXPECTED" ] && ! grep -q -- " ./$EXPECTED\$" "$CHECKSUMS_FILE"; then
+              MISSING="$MISSING $EXPECTED"
+            fi
+          done
+          if [ -n "$MISSING" ]; then
+            echo "::error title=Checksums Incomplete::No checksum recorded for:$MISSING"
+            exit 1
+          fi
 
           if ! grep -q -- "-macos" "$CHECKSUMS_FILE"; then
             echo "::error title=Checksums Incomplete::No binary checksum was recorded"

Sources/USBIPDCore/Device/USBDeviceCommunicatorImplementation.swift

@@ -192,12 +192,14 @@ public class USBDeviceCommunicatorImplementation: USBDeviceCommunicator, @unchec
         logger.debug("Executing control transfer for device \(device.busID)-\(device.deviceID), endpoint \(request.endpoint)")
 
         // Execute control transfer through IOKit interface
-        return try await interface.executeControlTransfer(
+        let result = try await interface.executeControlTransfer(
             endpoint: request.endpoint,
             setupPacket: request.setupPacket ?? Data(),
             transferBuffer: request.transferBuffer,
             timeout: request.timeout
         )
+        discardInterfaceIfDeviceGone(result, device: device, interfaceNumber: 0)
+        return result
     }
 
     // Compose the USB endpoint address IOKit expects.
@@ -226,12 +228,14 @@ public class USBDeviceCommunicatorImplementation: USBDeviceCommunicator, @unchec
         logger.debug("Executing bulk transfer for device \(device.busID)-\(device.deviceID), endpoint \(request.endpoint)")
 
         // Execute bulk transfer through IOKit interface
-        return try await interface.executeBulkTransfer(
+        let result = try await interface.executeBulkTransfer(
             endpoint: endpointAddress(for: request),
             data: request.transferBuffer,
             bufferLength: request.bufferLength,
             timeout: request.timeout
         )
+        discardInterfaceIfDeviceGone(result, device: device, interfaceNumber: 0)
+        return result
     }
 
     public func executeInterruptTransfer(device: USBDevice, request: USBRequestBlock) async throws -> USBTransferResult {
@@ -245,12 +249,14 @@ public class USBDeviceCommunicatorImplementation: USBDeviceCommunicator, @unchec
         logger.debug("Executing interrupt transfer for device \(device.busID)-\(device.deviceID), endpoint \(request.endpoint)")
 
         // Execute interrupt transfer through IOKit interface
-        return try await interface.executeInterruptTransfer(
+        let result = try await interface.executeInterruptTransfer(
             endpoint: endpointAddress(for: request),
             data: request.transferBuffer,
             bufferLength: request.bufferLength,
             timeout: request.timeout
         )
+        discardInterfaceIfDeviceGone(result, device: device, interfaceNumber: 0)
+        return result
     }
 
     public func executeIsochronousTransfer(device: USBDevice, request: USBRequestBlock) async throws -> USBTransferResult {
@@ -264,13 +270,15 @@ public class USBDeviceCommunicatorImplementation: USBDeviceCommunicator, @unchec
         logger.debug("Executing isochronous transfer for device \(device.busID)-\(device.deviceID), endpoint \(request.endpoint)")
 
         // Execute isochronous transfer through IOKit interface
-        return try await interface.executeIsochronousTransfer(
+        let result = try await interface.executeIsochronousTransfer(
             endpoint: endpointAddress(for: request),
             data: request.transferBuffer,
             bufferLength: request.bufferLength,
             startFrame: request.startFrame,
             numberOfPackets: max(request.numberOfPackets, 1)
         )
+        discardInterfaceIfDeviceGone(result, device: device, interfaceNumber: 0)
+        return result
     }
 
     // MARK: - Helper Methods
@@ -354,6 +362,55 @@ public class USBDeviceCommunicatorImplementation: USBDeviceCommunicator, @unchec
         return interface
     }
 
+    /// Whether a transfer result means the cached interface can no longer be used.
+    ///
+    /// `deviceGone` is what both `kIOReturnNoDevice` and `kIOReturnNotResponding` map
+    /// to. Either way the IOKit interface behind the handle is finished, and every
+    /// transfer through it will keep failing.
+    static func shouldDiscardInterface(after status: USBStatus) -> Bool {
+        return status == .deviceGone
+    }
+
+    /// Drop a cached interface whose device has gone, so the next request opens a new
+    /// one instead of reusing a handle that can only fail.
+    ///
+    /// Interfaces are opened once and kept. That is right while a device stays put, and
+    /// wrong the moment it does not: a device that disappears briefly — re-enumerating,
+    /// or an Android phone changing its USB configuration — left the daemon holding a
+    /// dead handle, and every subsequent transfer returned "no device" until the daemon
+    /// was restarted. Observed with a Pixel, where a fresh daemon worked immediately
+    /// while the running one never recovered.
+    private func discardInterfaceIfDeviceGone(
+        _ result: USBTransferResult,
+        device: USBDevice,
+        interfaceNumber: UInt8
+    ) {
+        guard USBDeviceCommunicatorImplementation.shouldDiscardInterface(after: result.status) else {
+            return
+        }
+
+        let deviceKey = deviceIdentifier(for: device)
+
+        // Held only long enough to take the interface out of the table. Letting it
+        // deallocate inside the lock would run its deinit — and so `close()`, and so
+        // IOKit calls — while every other transfer waited on that lock.
+        var discarded: IOKitUSBInterface?
+        interfaceLock.lock()
+        discarded = activeInterfaces[deviceKey]?.removeValue(forKey: interfaceNumber)
+        if activeInterfaces[deviceKey]?.isEmpty == true {
+            activeInterfaces.removeValue(forKey: deviceKey)
+        }
+        interfaceLock.unlock()
+
+        guard discarded != nil else { return }
+        discarded = nil
+
+        logger.warning("Device reported gone; discarding the cached interface so it is reopened", context: [
+            "device": deviceKey,
+            "interface": String(interfaceNumber)
+        ])
+    }
+
     // MARK: - Transfer Cancellation
 
     public func cancelAllTransfers(device: USBDevice, interfaceNumber: UInt8) async throws {

Tests/USBIPDCoreTests/Device/StaleInterfaceTests.swift

@@ -0,0 +1,50 @@
+// StaleInterfaceTests.swift
+// A cached interface must be dropped once its device is gone, and kept otherwise.
+
+import XCTest
+@testable import USBIPDCore
+@testable import Common
+
+final class StaleInterfaceTests: XCTestCase {
+
+    /// Interfaces are opened once and reused, which is right while a device stays put
+    /// and wrong the moment it does not. A device that disappeared briefly left the
+    /// daemon holding a dead IOKit handle, and every transfer through it failed with
+    /// "no device" until the daemon was restarted — observed with a Pixel, where a fresh
+    /// daemon worked immediately while the running one never recovered.
+    func testDeviceGoneDiscardsTheInterface() {
+        XCTAssertTrue(USBDeviceCommunicatorImplementation.shouldDiscardInterface(after: .deviceGone))
+    }
+
+    /// The other half, and the more important one. Reopening on any old failure would
+    /// throw away a perfectly good interface on every stall or timeout — and a timeout
+    /// is the ordinary outcome of a read on an endpoint with nothing to say, which this
+    /// project's own concurrency check produces on purpose.
+    func testOrdinaryFailuresKeepTheInterface() {
+        let keepers: [USBIPDCore.USBStatus] = [
+            .success,
+            .timeout,
+            .cancelled,
+            .stall,
+            .shortPacket,
+            .bufferError,
+            .invalidRequest,
+            .requestFailed
+        ]
+        for status in keepers {
+            XCTAssertFalse(
+                USBDeviceCommunicatorImplementation.shouldDiscardInterface(after: status),
+                "\(status) must not discard a usable interface")
+        }
+    }
+
+    /// Both IOKit codes that mean the device is finished map to the same status, so
+    /// covering `deviceGone` covers `kIOReturnNoDevice` and `kIOReturnNotResponding`.
+    func testBothIOKitDeviceGoneCodesMapToTheDiscardedStatus() {
+        XCTAssertEqual(USBErrorMapping.mapIOKitError(kIOReturnNoDevice), USBStatus.deviceGone.rawValue)
+        XCTAssertEqual(USBErrorMapping.mapIOKitError(kIOReturnNotResponding), USBStatus.deviceGone.rawValue)
+
+        // And a timeout does not, or the check above would be vacuous.
+        XCTAssertNotEqual(USBErrorMapping.mapIOKitError(kIOReturnTimeout), USBStatus.deviceGone.rawValue)
+    }
+}