Skip to content

Commit 5d8878b

Browse files
projectgusdpgeorge
authored andcommitted
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 micropython#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 <[email protected]>
1 parent 52a593c commit 5d8878b

File tree

2 files changed

+53
-0
lines changed

2 files changed

+53
-0
lines changed

shared/tinyusb/mp_usbd_runtime.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,15 @@ void mp_usbd_task_callback(mp_sched_node_t *node) {
501501
// Task function can be called manually to force processing of USB events
502502
// (mostly from USB-CDC serial port when blocking.)
503503
void mp_usbd_task(void) {
504+
#if MICROPY_PY_THREAD && !MICROPY_PY_THREAD_GIL
505+
if (!mp_thread_is_main_thread()) {
506+
// Avoid race with the scheduler callback by scheduling TinyUSB to run
507+
// on the main thread.
508+
mp_usbd_schedule_task();
509+
return;
510+
}
511+
#endif
512+
504513
if (in_usbd_task) {
505514
// If this exception triggers, it means a USB callback tried to do
506515
// something that itself became blocked on TinyUSB (most likely: read or

tests/thread/thread_stdin.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
# Test that having multiple threads block on stdin doesn't cause any issues.
2+
#
3+
# The test doesn't expect any input on stdin.
4+
#
5+
# This is a regression test for https://github.com/micropython/micropython/issues/15230
6+
# on rp2, but doubles as a general property to test across all ports.
7+
import sys
8+
import _thread
9+
10+
try:
11+
import select
12+
except ImportError:
13+
print("SKIP")
14+
raise SystemExit
15+
16+
17+
class StdinWaiter:
18+
def __init__(self):
19+
self._done = False
20+
21+
def wait_stdin(self, timeout_ms):
22+
poller = select.poll()
23+
poller.register(sys.stdin, select.POLLIN)
24+
poller.poll(timeout_ms)
25+
# Ignoring the poll result as we don't expect any input
26+
self._done = True
27+
28+
def is_done(self):
29+
return self._done
30+
31+
32+
thread_waiter = StdinWaiter()
33+
_thread.start_new_thread(thread_waiter.wait_stdin, (1000,))
34+
StdinWaiter().wait_stdin(1000)
35+
36+
# Spinning here is mostly not necessary but there is some inconsistency waking
37+
# the two threads, especially on CPython CI runners where the thread may not
38+
# have run yet. The actual delay is <20ms but spinning here instead of
39+
# sleep(0.1) means the test can run on MP builds without float support.
40+
while not thread_waiter.is_done():
41+
pass
42+
43+
# The background thread should have completed its wait by now.
44+
print(thread_waiter.is_done())

0 commit comments

Comments
 (0)