interrupt: Split set_handler context.

Since introducing the ctx pointer, the handler is now two words, so setting it can
race with the interrupt firing. On race it's possible for the new handler to be
alled with the old ctx pointer or viceversa.

Rather than documenting this, it's better to split the function in two to make it
obvious to the user that it's not atomic. The user can use a critical section, or
disable/enable the interrupt to avoid races if this is a concern.

10 files changed, 41 insertions, 42 deletions

diff --git a/embassy-nrf/src/gpiote.rs b/embassy-nrf/src/gpiote.rs
index 592ca82a7..01e61bd5c 100644
--- a/embassy-nrf/src/gpiote.rs
+++ b/embassy-nrf/src/gpiote.rs
@@ -118,7 +118,7 @@ impl Gpiote {
         // Enable interrupts
         gpiote.events_port.write(|w| w);
         gpiote.intenset.write(|w| w.port().set());
-        irq.set_handler(Self::on_irq, core::ptr::null_mut());
+        irq.set_handler(Self::on_irq);
         irq.unpend();
         irq.enable();
 
diff --git a/embassy-nrf/src/qspi.rs b/embassy-nrf/src/qspi.rs
index 902d6511b..300b32ad5 100644
--- a/embassy-nrf/src/qspi.rs
+++ b/embassy-nrf/src/qspi.rs
@@ -146,7 +146,7 @@ impl Qspi {
         SIGNAL.reset();
         qspi.intenset.write(|w| w.ready().set());
 
-        irq.set_handler(irq_handler, core::ptr::null_mut());
+        irq.set_handler(irq_handler);
         irq.unpend();
         irq.enable();
 
diff --git a/embassy-nrf/src/rtc.rs b/embassy-nrf/src/rtc.rs
index 867c710c3..7a79580cc 100644
--- a/embassy-nrf/src/rtc.rs
+++ b/embassy-nrf/src/rtc.rs
@@ -109,13 +109,11 @@ impl<T: Instance> RTC<T> {
         // Wait for clear
         while self.rtc.counter.read().bits() != 0 {}
 
-        self.irq.set_handler(
-            |ptr| unsafe {
-                let this = &*(ptr as *const () as *const Self);
-                this.on_interrupt();
-            },
-            self as *const _ as *mut _,
-        );
+        self.irq.set_handler(|ptr| unsafe {
+            let this = &*(ptr as *const () as *const Self);
+            this.on_interrupt();
+        });
+        self.irq.set_handler_context(self as *const _ as *mut _);
         self.irq.unpend();
         self.irq.enable();
     }
diff --git a/embassy-nrf/src/uarte.rs b/embassy-nrf/src/uarte.rs
index f029276b2..07718a1dd 100644
--- a/embassy-nrf/src/uarte.rs
+++ b/embassy-nrf/src/uarte.rs
@@ -119,7 +119,7 @@ where
             .write(|w| w.endtx().set().txstopped().set().endrx().set().rxto().set());
 
         // Register ISR
-        irq.set_handler(Self::on_irq, core::ptr::null_mut());
+        irq.set_handler(Self::on_irq);
         irq.unpend();
         irq.enable();
 
diff --git a/embassy-nrf/src/util/peripheral.rs b/embassy-nrf/src/util/peripheral.rs
index 8cefaeef8..bf78d7760 100644
--- a/embassy-nrf/src/util/peripheral.rs
+++ b/embassy-nrf/src/util/peripheral.rs
@@ -33,16 +33,14 @@ impl<S: PeripheralState> PeripheralMutex<S> {
         irq.disable();
         compiler_fence(Ordering::SeqCst);
 
-        irq.set_handler(
-            |p| {
-                // Safety: it's OK to get a &mut to the state, since
-                // - We're in the IRQ, no one else can't preempt us
-                // - We can't have preempted a with() call because the irq is disabled during it.
-                let state = unsafe { &mut *(p as *mut S) };
-                state.on_interrupt();
-            },
-            state.get() as *mut (),
-        );
+        irq.set_handler(|p| {
+            // Safety: it's OK to get a &mut to the state, since
+            // - We're in the IRQ, no one else can't preempt us
+            // - We can't have preempted a with() call because the irq is disabled during it.
+            let state = unsafe { &mut *(p as *mut S) };
+            state.on_interrupt();
+        });
+        irq.set_handler_context(state.get() as *mut ());
 
         // Safety: it's OK to get a &mut to the state, since the irq is disabled.
         let state = unsafe { &mut *state.get() };
diff --git a/embassy-stm32f4/src/rtc.rs b/embassy-stm32f4/src/rtc.rs
index da7702876..21589f5d6 100644
--- a/embassy-stm32f4/src/rtc.rs
+++ b/embassy-stm32f4/src/rtc.rs
@@ -92,13 +92,11 @@ impl<T: Instance> RTC<T> {
         self.rtc.set_compare(0, 0x8000);
         self.rtc.set_compare_interrupt(0, true);
 
-        self.irq.set_handler(
-            |ptr| unsafe {
-                let this = &*(ptr as *const () as *const Self);
-                this.on_interrupt();
-            },
-            self as *const _ as *mut _,
-        );
+        self.irq.set_handler(|ptr| unsafe {
+            let this = &*(ptr as *const () as *const Self);
+            this.on_interrupt();
+        });
+        self.irq.set_handler_context(self as *const _ as *mut _);
         self.irq.unpend();
         self.irq.enable();
 
diff --git a/embassy-stm32f4/src/serial.rs b/embassy-stm32f4/src/serial.rs
index d84d8cb01..669d7856c 100644
--- a/embassy-stm32f4/src/serial.rs
+++ b/embassy-stm32f4/src/serial.rs
@@ -70,9 +70,9 @@ impl Serial<USART1, Stream7<DMA2>, Stream2<DMA2>> {
         let (usart, _) = serial.release();
 
         // Register ISR
-        tx_int.set_handler(Self::on_tx_irq, core::ptr::null_mut());
-        rx_int.set_handler(Self::on_rx_irq, core::ptr::null_mut());
-        usart_int.set_handler(Self::on_rx_irq, core::ptr::null_mut());
+        tx_int.set_handler(Self::on_tx_irq);
+        rx_int.set_handler(Self::on_rx_irq);
+        usart_int.set_handler(Self::on_rx_irq);
         // usart_int.unpend();
         // usart_int.enable();
 
diff --git a/embassy/src/executor/mod.rs b/embassy/src/executor/mod.rs
index 6c42764bb..be0d1b9e7 100644
--- a/embassy/src/executor/mod.rs
+++ b/embassy/src/executor/mod.rs
@@ -244,13 +244,11 @@ impl<I: Interrupt> IrqExecutor<I> {
 
         init(unsafe { self.inner.spawner() });
 
-        self.irq.set_handler(
-            |ctx| unsafe {
-                let executor = &*(ctx as *const raw::Executor);
-                executor.run_queued();
-            },
-            &self.inner as *const _ as _,
-        );
+        self.irq.set_handler(|ctx| unsafe {
+            let executor = &*(ctx as *const raw::Executor);
+            executor.run_queued();
+        });
+        self.irq.set_handler_context(&self.inner as *const _ as _);
         self.irq.enable();
     }
 }
diff --git a/embassy/src/interrupt.rs b/embassy/src/interrupt.rs
index 9106def5f..53413d41d 100644
--- a/embassy/src/interrupt.rs
+++ b/embassy/src/interrupt.rs
@@ -38,10 +38,9 @@ pub unsafe trait Interrupt {
     #[doc(hidden)]
     unsafe fn __handler(&self) -> &'static Handler;
 
-    fn set_handler(&self, func: unsafe fn(*mut ()), ctx: *mut ()) {
+    fn set_handler(&self, func: unsafe fn(*mut ())) {
         let handler = unsafe { self.__handler() };
         handler.func.store(func as *mut (), Ordering::Release);
-        handler.ctx.store(ctx, Ordering::Release);
     }
 
     fn remove_handler(&self) {
@@ -49,6 +48,11 @@ pub unsafe trait Interrupt {
         handler.func.store(ptr::null_mut(), Ordering::Release);
     }
 
+    fn set_handler_context(&self, ctx: *mut ()) {
+        let handler = unsafe { self.__handler() };
+        handler.ctx.store(ctx, Ordering::Release);
+    }
+
     #[inline]
     fn enable(&self) {
         unsafe {
diff --git a/embassy/src/util/signal.rs b/embassy/src/util/signal.rs
index 9fcfa7ac1..0abcc75f3 100644
--- a/embassy/src/util/signal.rs
+++ b/embassy/src/util/signal.rs
@@ -93,7 +93,8 @@ impl<'a, I: Interrupt> Drop for InterruptFuture<'a, I> {
 impl<'a, I: Interrupt> InterruptFuture<'a, I> {
     pub fn new(interrupt: &'a mut I) -> Self {
         interrupt.disable();
-        interrupt.set_handler(Self::interrupt_handler, ptr::null_mut());
+        interrupt.set_handler(Self::interrupt_handler);
+        interrupt.set_handler_context(ptr::null_mut());
         interrupt.unpend();
         interrupt.enable();
 
@@ -121,8 +122,10 @@ impl<'a, I: Interrupt> Future for InterruptFuture<'a, I> {
 
     fn poll(self: core::pin::Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<()> {
         let s = unsafe { self.get_unchecked_mut() };
-        let ctx = unsafe { executor::raw::task_from_waker(&cx.waker()).cast().as_ptr() };
-        s.interrupt.set_handler(Self::interrupt_handler, ctx);
+        s.interrupt.set_handler(Self::interrupt_handler);
+        s.interrupt.set_handler_context(unsafe {
+            executor::raw::task_from_waker(&cx.waker()).cast().as_ptr()
+        });
         if s.interrupt.is_enabled() {
             Poll::Pending
         } else {