Skip to content

Commit 69993da

Browse files
projectgusdpgeorge
authored andcommitted
rp2/modmachine: Add mutual exclusion for machine.lightsleep().
There's no specified behaviour for what should happen if both CPUs call `lightsleep()` together, but the latest changes could cause a permanent hang due to a race in the timer cleanup code. Add a flag to prevent hangs if two threads accidentally lightsleep, at least. This allows the new lightsleep test to pass on RPI_PICO and RPI_PICO2, and even have much tighter time deltas. However, the test still fails on wireless boards where the lwIP tick wakes them up too frequently. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <[email protected]>
1 parent 977fd94 commit 69993da

File tree

2 files changed

+34
-5
lines changed

2 files changed

+34
-5
lines changed

ports/rp2/modmachine.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,16 @@ static void mp_machine_lightsleep(size_t n_args, const mp_obj_t *args) {
173173
}
174174
#endif
175175

176+
#if MICROPY_PY_THREAD
177+
static bool in_lightsleep;
178+
if (in_lightsleep) {
179+
// The other CPU is also in machine.lightsleep()
180+
MICROPY_END_ATOMIC_SECTION(my_interrupts);
181+
return;
182+
}
183+
in_lightsleep = true;
184+
#endif
185+
176186
#if MICROPY_HW_ENABLE_USBDEV
177187
// Only disable the USB clock if a USB host has not configured the device
178188
// or if going to DORMANT mode.
@@ -320,6 +330,12 @@ static void mp_machine_lightsleep(size_t n_args, const mp_obj_t *args) {
320330
#endif
321331
#endif
322332
}
333+
334+
#if MICROPY_PY_THREAD
335+
// Clearing the flag here is atomic, and we know we're the ones who set it
336+
// (higher up, inside the critical section)
337+
in_lightsleep = false;
338+
#endif
323339
}
324340

325341
NORETURN static void mp_machine_deepsleep(size_t n_args, const mp_obj_t *args) {

tests/ports/rp2/rp2_lightsleep_thread.py

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,25 @@ def test_cpu0_sleeping(self):
4242

4343
def test_cpu0_also_lightsleep(self):
4444
_thread.start_new_thread(self.thread_entry, ())
45-
time.sleep(0.050) # account for any delay in starting the thread
45+
time.sleep_ms(50) # account for any delay in starting the thread
4646
self.thread_entry(False) # does the same lightsleep loop, doesn't set the done flag
47-
self.assertTrue(self.thread_done)
48-
# only one thread can actually be in lightsleep at a time to avoid races, so the total
49-
# runtime is doubled by doing it on both CPUs
50-
self.assertAlmostEqual(self.elapsed_ms(), IDEAL_RUNTIME * 2, delta=IDEAL_RUNTIME)
47+
while not self.thread_done:
48+
time.sleep_ms(10)
49+
#
50+
# Only one thread can actually be in lightsleep at a time to avoid
51+
# races, but otherwise the behaviour when both threads call lightsleep()
52+
# is unspecified.
53+
#
54+
# Currently, the other thread will return immediately if one is already
55+
# in lightsleep. Therefore, runtime can be between IDEAL_RUNTIME and
56+
# IDEAL_RUNTIME * 2 depending on how many times the calls to lightsleep() race
57+
# each other.
58+
#
59+
# Note this test case is really only here to ensure that the rp2 hasn't
60+
# hung or failed to sleep at all - not to verify any correct behaviour
61+
# when there's a race to call lightsleep().
62+
self.assertGreaterEqual(self.elapsed_ms(), IDEAL_RUNTIME - MAX_DELTA)
63+
self.assertLessEqual(self.elapsed_ms(), IDEAL_RUNTIME * 2 + MAX_DELTA)
5164

5265

5366
if __name__ == "__main__":

0 commit comments

Comments
 (0)