diff options
author | Angus Gratton <angus@redyak.com.au> | 2024-09-04 17:18:38 +1000 |
---|---|---|
committer | Damien George <damien@micropython.org> | 2024-09-19 13:17:01 +1000 |
commit | 5d8878b582b8b68d19ab02adfe32d683d5ea512f (patch) | |
tree | 672939c19f6dbf70088aa5459c108af5fbfe75d8 | |
parent | 52a593cdb14ed732b5580bbed39c0325815adedf (diff) | |
download | micropython-5d8878b582b8b68d19ab02adfe32d683d5ea512f.tar.gz micropython-5d8878b582b8b68d19ab02adfe32d683d5ea512f.zip |
shared/tinyusb: Only run TinyUSB on the main thread if GIL is disabled.
If GIL is disabled then there's threat of a race condition if some other
code specifically requests USB processing (i.e. to unblock stdio), while
a scheduled TinyUSB callback is already running on another thread.
Relies on the change in the parent commit, where scheduler is restricted
to main thread if GIL is disabled.
Fixes #15390 - "TinyUSB callback can't recurse" exceptions on rp2 when
using _thread module and USB serial I/O.
Adds a unit test for stdin functioning correctly in threads (fails on rp2
port without this fix).
This work was funded through GitHub Sponsors.
Signed-off-by: Angus Gratton <angus@redyak.com.au>
-rw-r--r-- | shared/tinyusb/mp_usbd_runtime.c | 9 | ||||
-rw-r--r-- | tests/thread/thread_stdin.py | 44 |
2 files changed, 53 insertions, 0 deletions
diff --git a/shared/tinyusb/mp_usbd_runtime.c b/shared/tinyusb/mp_usbd_runtime.c index fe28a4a727..4955382577 100644 --- a/shared/tinyusb/mp_usbd_runtime.c +++ b/shared/tinyusb/mp_usbd_runtime.c @@ -501,6 +501,15 @@ void mp_usbd_task_callback(mp_sched_node_t *node) { // Task function can be called manually to force processing of USB events // (mostly from USB-CDC serial port when blocking.) void mp_usbd_task(void) { + #if MICROPY_PY_THREAD && !MICROPY_PY_THREAD_GIL + if (!mp_thread_is_main_thread()) { + // Avoid race with the scheduler callback by scheduling TinyUSB to run + // on the main thread. + mp_usbd_schedule_task(); + return; + } + #endif + if (in_usbd_task) { // If this exception triggers, it means a USB callback tried to do // something that itself became blocked on TinyUSB (most likely: read or diff --git a/tests/thread/thread_stdin.py b/tests/thread/thread_stdin.py new file mode 100644 index 0000000000..a469933f19 --- /dev/null +++ b/tests/thread/thread_stdin.py @@ -0,0 +1,44 @@ +# Test that having multiple threads block on stdin doesn't cause any issues. +# +# The test doesn't expect any input on stdin. +# +# This is a regression test for https://github.com/micropython/micropython/issues/15230 +# on rp2, but doubles as a general property to test across all ports. +import sys +import _thread + +try: + import select +except ImportError: + print("SKIP") + raise SystemExit + + +class StdinWaiter: + def __init__(self): + self._done = False + + def wait_stdin(self, timeout_ms): + poller = select.poll() + poller.register(sys.stdin, select.POLLIN) + poller.poll(timeout_ms) + # Ignoring the poll result as we don't expect any input + self._done = True + + def is_done(self): + return self._done + + +thread_waiter = StdinWaiter() +_thread.start_new_thread(thread_waiter.wait_stdin, (1000,)) +StdinWaiter().wait_stdin(1000) + +# Spinning here is mostly not necessary but there is some inconsistency waking +# the two threads, especially on CPython CI runners where the thread may not +# have run yet. The actual delay is <20ms but spinning here instead of +# sleep(0.1) means the test can run on MP builds without float support. +while not thread_waiter.is_done(): + pass + +# The background thread should have completed its wait by now. +print(thread_waiter.is_done()) |