usb: Fix USB device library transfer thread safety

micropython/usb/README.md

@@ -134,3 +134,15 @@ USB MIDI devices in MicroPython.
 
 The example [midi_example.py](examples/device/midi_example.py) demonstrates how
 to create a simple MIDI device to send MIDI data to and from the USB host.
+
+### Limitations
+
+#### Buffer thread safety
+
+The internal Buffer class that's used by most of the USB device classes expects data
+to be written to it (i.e. sent to the host) by only one thread. Bytes may be
+lost from the USB transfers if more than one thread (or a thread and a callback)
+try to write to the buffer simultaneously.
+
+If writing USB data from multiple sources, your code may need to add
+synchronisation (i.e. locks).

micropython/usb/usb-device/manifest.py

@@ -1,2 +1,2 @@
-metadata(version="0.1.0")
+metadata(version="0.1.1")
 package("usb")

micropython/usb/usb-device/usb/device/core.py

@@ -8,6 +8,14 @@
 import machine
 import struct
 
+try:
+    from _thread import get_ident
+except ImportError:
+
+    def get_ident():
+        return 0  # Placeholder, for no threading support
+
+
 _EP_IN_FLAG = const(1 << 7)
 
 # USB descriptor types
@@ -76,6 +84,8 @@ def __init__(self):
         self._itfs = {}  # Mapping from interface number to interface object, set by init()
         self._eps = {}  # Mapping from endpoint address to interface object, set by _open_cb()
         self._ep_cbs = {}  # Mapping from endpoint address to Optional[xfer callback]
+        self._cb_thread = None  # Thread currently running endpoint callback
+        self._cb_ep = None  # Endpoint number currently running callback
         self._usbd = machine.USBDevice()  # low-level API
 
     def init(self, *itfs, **kwargs):
@@ -242,8 +252,8 @@ def active(self, *optional_value):
         return self._usbd.active(*optional_value)
 
     def _open_itf_cb(self, desc):
-        # Singleton callback from TinyUSB custom class driver, when USB host does
-        # Set Configuration. Called once per interface or IAD.
+        # Callback from TinyUSB lower layer, when USB host does Set
+        # Configuration. Called once per interface or IAD.
 
         # Note that even if the configuration descriptor contains an IAD, 'desc'
         # starts from the first interface descriptor in the IAD and not the IAD
@@ -281,7 +291,7 @@ def _open_itf_cb(self, desc):
             itf.on_open()
 
     def _reset_cb(self):
-        # Callback when the USB device is reset by the host
+        # TinyUSB lower layer callback when the USB device is reset by the host
 
         # Allow interfaces to respond to the reset
         for itf in self._itfs.values():
@@ -292,13 +302,13 @@ def _reset_cb(self):
         self._ep_cbs = {}
 
     def _submit_xfer(self, ep_addr, data, done_cb=None):
-        # Singleton function to submit a USB transfer (of any type except control).
+        # Submit a USB transfer (of any type except control) to TinyUSB lower layer.
         #
         # Generally, drivers should call Interface.submit_xfer() instead. See
         # that function for documentation about the possible parameter values.
         if ep_addr not in self._eps:
             raise ValueError("ep_addr")
-        if self._ep_cbs[ep_addr]:
+        if self._xfer_pending(ep_addr):
             raise RuntimeError("xfer_pending")
 
         # USBDevice callback may be called immediately, before Python execution
@@ -308,15 +318,32 @@ def _submit_xfer(self, ep_addr, data, done_cb=None):
         self._ep_cbs[ep_addr] = done_cb or True
         return self._usbd.submit_xfer(ep_addr, data)
 
+    def _xfer_pending(self, ep_addr):
+        # Returns True if a transfer is pending on this endpoint.
+        #
+        # Generally, drivers should call Interface.xfer_pending() instead. See that
+        # function for more documentation.
+        return self._ep_cbs[ep_addr] or (self._cb_ep == ep_addr and self._cb_thread != get_ident())
+
     def _xfer_cb(self, ep_addr, result, xferred_bytes):
-        # Singleton callback from TinyUSB custom class driver when a transfer completes.
+        # Callback from TinyUSB lower layer when a transfer completes.
         cb = self._ep_cbs.get(ep_addr, None)
+        self._cb_thread = get_ident()
+        self._cb_ep = ep_addr  # Track while callback is running
         self._ep_cbs[ep_addr] = None
-        if callable(cb):
-            cb(ep_addr, result, xferred_bytes)
+
+        # In most cases, 'cb' is a callback function for the transfer. Can also be:
+        # - True (for a transfer with no callback)
+        # - None (TinyUSB callback arrived for invalid endpoint, or no transfer.
+        #   Generally unlikely, but may happen in transient states.)
+        try:
+            if callable(cb):
+                cb(ep_addr, result, xferred_bytes)
+        finally:
+            self._cb_ep = None
 
     def _control_xfer_cb(self, stage, request):
-        # Singleton callback from TinyUSB custom class driver when a control
+        # Callback from TinyUSB lower layer when a control
         # transfer is in progress.
         #
         # stage determines appropriate responses (possible values
@@ -528,7 +555,12 @@ def xfer_pending(self, ep_addr):
         # Return True if a transfer is already pending on ep_addr.
         #
         # Only one transfer can be submitted at a time.
-        return _dev and bool(_dev._ep_cbs[ep_addr])
+        #
+        # The transfer is marked pending while a completion callback is running
+        # for that endpoint, unless this function is called from the callback
+        # itself. This makes it simple to submit a new transfer from the
+        # completion callback.
+        return _dev and _dev._xfer_pending(ep_addr)
 
     def submit_xfer(self, ep_addr, data, done_cb=None):
         # Submit a USB transfer (of any type except control)