From 97598968eba9a199500d319ca1fcd8a86a4b9bb7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?D=C3=A1niel=20Buga?= <bugadani@gmail.com>
Date: Thu, 19 Dec 2024 08:53:48 +0100
Subject: [PATCH] GPIO: prevent woken task interrupting GPIO handler (#2838)

* Clear interrupt bit before allowing waker to wake a task

* Add test
---
 esp-hal/src/gpio/mod.rs         |  7 +++++--
 esp-hal/src/interrupt/riscv.rs  |  1 +
 esp-hal/src/interrupt/xtensa.rs |  1 +
 esp-hal/src/uart.rs             |  2 +-
 hil-test/tests/gpio.rs          | 29 +++++++++++++++++++++++++++++
 5 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/esp-hal/src/gpio/mod.rs b/esp-hal/src/gpio/mod.rs
index 9d7648784..9f23c9108 100644
--- a/esp-hal/src/gpio/mod.rs
+++ b/esp-hal/src/gpio/mod.rs
@@ -930,8 +930,11 @@ fn handle_pin_interrupts(user_handler: fn()) {
             intr_bits -= 1 << pin_pos;
 
             let pin_nr = pin_pos as u8 + bank.offset();
-            asynch::PIN_WAKERS[pin_nr as usize].wake();
-            set_int_enable(pin_nr, Some(0), 0, false);
+
+            crate::interrupt::free(|| {
+                asynch::PIN_WAKERS[pin_nr as usize].wake();
+                set_int_enable(pin_nr, Some(0), 0, false);
+            });
         }
 
         bank.write_interrupt_status_clear(intrs);
diff --git a/esp-hal/src/interrupt/riscv.rs b/esp-hal/src/interrupt/riscv.rs
index 1aaa3eddd..69170a7e2 100644
--- a/esp-hal/src/interrupt/riscv.rs
+++ b/esp-hal/src/interrupt/riscv.rs
@@ -12,6 +12,7 @@
 //! interrupt15() => Priority::Priority15
 //! ```
 
+pub(crate) use esp_riscv_rt::riscv::interrupt::free;
 pub use esp_riscv_rt::TrapFrame;
 use riscv::register::{mcause, mtvec};
 
diff --git a/esp-hal/src/interrupt/xtensa.rs b/esp-hal/src/interrupt/xtensa.rs
index 7bf72e855..22079a478 100644
--- a/esp-hal/src/interrupt/xtensa.rs
+++ b/esp-hal/src/interrupt/xtensa.rs
@@ -1,6 +1,7 @@
 //! Interrupt handling
 
 use xtensa_lx::interrupt;
+pub(crate) use xtensa_lx::interrupt::free;
 use xtensa_lx_rt::exception::Context;
 
 pub use self::vectored::*;
diff --git a/esp-hal/src/uart.rs b/esp-hal/src/uart.rs
index 546f0448e..7d75f7cf7 100644
--- a/esp-hal/src/uart.rs
+++ b/esp-hal/src/uart.rs
@@ -904,7 +904,7 @@ where
             cfg_if::cfg_if! {
                 if #[cfg(esp32)] {
                     // https://docs.espressif.com/projects/esp-chip-errata/en/latest/esp32/03-errata-description/esp32/cpu-subsequent-access-halted-when-get-interrupted.html
-                    xtensa_lx::interrupt::free(|| {
+                    crate::interrupt::free(|| {
                         *byte = fifo.read().rxfifo_rd_byte().bits();
                     });
                 } else {
diff --git a/hil-test/tests/gpio.rs b/hil-test/tests/gpio.rs
index 7ff28cb4c..af03c2329 100644
--- a/hil-test/tests/gpio.rs
+++ b/hil-test/tests/gpio.rs
@@ -399,4 +399,33 @@ mod tests {
 
         _ = Input::new(pin, Pull::Down);
     }
+
+    #[test]
+    fn interrupt_executor_is_not_frozen(ctx: Context) {
+        use esp_hal::interrupt::{software::SoftwareInterrupt, Priority};
+        use esp_hal_embassy::InterruptExecutor;
+        use static_cell::StaticCell;
+
+        static INTERRUPT_EXECUTOR: StaticCell<InterruptExecutor<1>> = StaticCell::new();
+        let interrupt_executor = INTERRUPT_EXECUTOR.init(InterruptExecutor::new(unsafe {
+            SoftwareInterrupt::<1>::steal()
+        }));
+
+        let spawner = interrupt_executor.start(Priority::max());
+
+        spawner.must_spawn(test_task(ctx.test_gpio1.degrade()));
+
+        #[embassy_executor::task]
+        async fn test_task(pin: AnyPin) {
+            let mut pin = Input::new(pin, Pull::Down);
+
+            // This line must return, even if the executor
+            // is running at a higher priority than the GPIO handler.
+            pin.wait_for_low().await;
+
+            embedded_test::export::check_outcome(());
+        }
+
+        loop {}
+    }
 }