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 <angus@redyak.com.au>

2 files changed, 34 insertions, 5 deletions

diff --git a/ports/rp2/modmachine.c b/ports/rp2/modmachine.c
index fd2912c80..3625d404c 100644
--- a/ports/rp2/modmachine.c
+++ b/ports/rp2/modmachine.c
@@ -173,6 +173,16 @@ static void mp_machine_lightsleep(size_t n_args, const mp_obj_t *args) {
     }
     #endif
 
+    #if MICROPY_PY_THREAD
+    static bool in_lightsleep;
+    if (in_lightsleep) {
+        // The other CPU is also in machine.lightsleep()
+        MICROPY_END_ATOMIC_SECTION(my_interrupts);
+        return;
+    }
+    in_lightsleep = true;
+    #endif
+
     #if MICROPY_HW_ENABLE_USBDEV
     // Only disable the USB clock if a USB host has not configured the device
     // or if going to DORMANT mode.
@@ -320,6 +330,12 @@ static void mp_machine_lightsleep(size_t n_args, const mp_obj_t *args) {
         #endif
         #endif
     }
+
+    #if MICROPY_PY_THREAD
+    // Clearing the flag here is atomic, and we know we're the ones who set it
+    // (higher up, inside the critical section)
+    in_lightsleep = false;
+    #endif
 }
 
 NORETURN static void mp_machine_deepsleep(size_t n_args, const mp_obj_t *args) {
diff --git a/tests/ports/rp2/rp2_lightsleep_thread.py b/tests/ports/rp2/rp2_lightsleep_thread.py
index 2aaa34bbe..494ead422 100644
--- a/tests/ports/rp2/rp2_lightsleep_thread.py
+++ b/tests/ports/rp2/rp2_lightsleep_thread.py
@@ -42,12 +42,25 @@ class LightSleepInThread(unittest.TestCase):
 
     def test_cpu0_also_lightsleep(self):
         _thread.start_new_thread(self.thread_entry, ())
-        time.sleep(0.050)  # account for any delay in starting the thread
+        time.sleep_ms(50)  # account for any delay in starting the thread
         self.thread_entry(False)  # does the same lightsleep loop, doesn't set the done flag
-        self.assertTrue(self.thread_done)
-        # only one thread can actually be in lightsleep at a time to avoid races, so the total
-        # runtime is doubled by doing it on both CPUs
-        self.assertAlmostEqual(self.elapsed_ms(), IDEAL_RUNTIME * 2, delta=IDEAL_RUNTIME)
+        while not self.thread_done:
+            time.sleep_ms(10)
+        #
+        # Only one thread can actually be in lightsleep at a time to avoid
+        # races, but otherwise the behaviour when both threads call lightsleep()
+        # is unspecified.
+        #
+        # Currently, the other thread will return immediately if one is already
+        # in lightsleep. Therefore, runtime can be between IDEAL_RUNTIME and
+        # IDEAL_RUNTIME * 2 depending on how many times the calls to lightsleep() race
+        # each other.
+        #
+        # Note this test case is really only here to ensure that the rp2 hasn't
+        # hung or failed to sleep at all - not to verify any correct behaviour
+        # when there's a race to call lightsleep().
+        self.assertGreaterEqual(self.elapsed_ms(), IDEAL_RUNTIME - MAX_DELTA)
+        self.assertLessEqual(self.elapsed_ms(), IDEAL_RUNTIME * 2 + MAX_DELTA)
 
 
 if __name__ == "__main__":