Skip to content

Commit ac5e0b9

Browse files
committed
rp2/mpthreadport: Fix race with IRQ when entering atomic section.
Prior to this commit there is a potential deadlock in mp_thread_begin_atomic_section(), when obtaining the atomic_mutex, in the following situation: - main thread calls mp_thread_begin_atomic_section() (for whatever reason, doesn't matter) - the second core is running so the main thread grabs the mutex via the call mp_thread_mutex_lock(&atomic_mutex, 1), and this succeeds - before the main thread has a chance to run save_and_disable_interrupts() a USB IRQ comes in and the main thread jumps off to process this IRQ - that USB processing triggers a call to the dcd_event_handler() wrapper from commit bcbdee2 - that then calls mp_sched_schedule_node() - that then attempts to obtain the atomic section, calling mp_thread_begin_atomic_section() - that call then blocks trying to obtain atomic_mutex - core0 is now deadlocked on itself, because the main thread has the mutex but the IRQ handler (which preempted the main thread) is blocked waiting for the mutex, which will never be free The solution in this commit is to use mutex enter/exit functions that also atomically disable/restore interrupts. Fixes issues micropython#12980 and micropython#13288. Signed-off-by: Damien George <[email protected]>
1 parent 61b8361 commit ac5e0b9

File tree

1 file changed

+10
-9
lines changed

1 file changed

+10
-9
lines changed

ports/rp2/mpthreadport.c

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "py/mpthread.h"
3131
#include "pico/stdlib.h"
3232
#include "pico/multicore.h"
33+
#include "mutex_extra.h"
3334

3435
#if MICROPY_PY_THREAD
3536

@@ -45,31 +46,31 @@ STATIC uint32_t *core1_stack = NULL;
4546
STATIC size_t core1_stack_num_words = 0;
4647

4748
// Thread mutex.
48-
STATIC mp_thread_mutex_t atomic_mutex;
49+
STATIC mutex_t atomic_mutex;
4950

5051
uint32_t mp_thread_begin_atomic_section(void) {
5152
if (core1_entry) {
5253
// When both cores are executing, we also need to provide
5354
// full mutual exclusion.
54-
mp_thread_mutex_lock(&atomic_mutex, 1);
55+
return mutex_enter_blocking_and_disable_interrupts(&atomic_mutex);
56+
} else {
57+
return save_and_disable_interrupts();
5558
}
56-
57-
return save_and_disable_interrupts();
5859
}
5960

6061
void mp_thread_end_atomic_section(uint32_t state) {
61-
restore_interrupts(state);
62-
63-
if (core1_entry) {
64-
mp_thread_mutex_unlock(&atomic_mutex);
62+
if (atomic_mutex.owner != LOCK_INVALID_OWNER_ID) {
63+
mutex_exit_and_restore_interrupts(&atomic_mutex, state);
64+
} else {
65+
restore_interrupts(state);
6566
}
6667
}
6768

6869
// Initialise threading support.
6970
void mp_thread_init(void) {
7071
assert(get_core_num() == 0);
7172

72-
mp_thread_mutex_init(&atomic_mutex);
73+
mutex_init(&atomic_mutex);
7374

7475
// Allow MICROPY_BEGIN_ATOMIC_SECTION to be invoked from core1.
7576
multicore_lockout_victim_init();

0 commit comments

Comments
 (0)