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.

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();
 

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();
 

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(
+        self.irq.set_handler(|ptr| unsafe {
-            |ptr| unsafe {
+            let this = &*(ptr as *const () as *const Self);
-                let this = &*(ptr as *const () as *const Self);
+            this.on_interrupt();
-                this.on_interrupt();
+        });
-            },
+        self.irq.set_handler_context(self as *const _ as *mut _);
-            self as *const _ as *mut _,
-        );
         self.irq.unpend();
         self.irq.enable();
     }

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();
 

embassy-nrf/src/util/peripheral.rs

@@ -33,16 +33,14 @@ impl<S: PeripheralState> PeripheralMutex<S> {
         irq.disable();
         compiler_fence(Ordering::SeqCst);
 
-        irq.set_handler(
+        irq.set_handler(|p| {
-            |p| {
+            // Safety: it's OK to get a &mut to the state, since
-                // 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'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.
-                // - We can't have preempted a with() call because the irq is disabled during it.
+            let state = unsafe { &mut *(p as *mut S) };
-                let state = unsafe { &mut *(p as *mut S) };
+            state.on_interrupt();
-                state.on_interrupt();
+        });
-            },
+        irq.set_handler_context(state.get() as *mut ());
-            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() };

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(
+        self.irq.set_handler(|ptr| unsafe {
-            |ptr| unsafe {
+            let this = &*(ptr as *const () as *const Self);
-                let this = &*(ptr as *const () as *const Self);
+            this.on_interrupt();
-                this.on_interrupt();
+        });
-            },
+        self.irq.set_handler_context(self as *const _ as *mut _);
-            self as *const _ as *mut _,
-        );
         self.irq.unpend();
         self.irq.enable();
 

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());
+        tx_int.set_handler(Self::on_tx_irq);
-        rx_int.set_handler(Self::on_rx_irq, core::ptr::null_mut());
+        rx_int.set_handler(Self::on_rx_irq);
-        usart_int.set_handler(Self::on_rx_irq, core::ptr::null_mut());
+        usart_int.set_handler(Self::on_rx_irq);
         // usart_int.unpend();
         // usart_int.enable();
 

embassy/src/executor/mod.rs

@@ -244,13 +244,11 @@ impl<I: Interrupt> IrqExecutor<I> {
 
         init(unsafe { self.inner.spawner() });
 
-        self.irq.set_handler(
+        self.irq.set_handler(|ctx| unsafe {
-            |ctx| unsafe {
+            let executor = &*(ctx as *const raw::Executor);
-                let executor = &*(ctx as *const raw::Executor);
+            executor.run_queued();
-                executor.run_queued();
+        });
-            },
+        self.irq.set_handler_context(&self.inner as *const _ as _);
-            &self.inner as *const _ as _,
-        );
         self.irq.enable();
     }
 }

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 {

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);
-        s.interrupt.set_handler(Self::interrupt_handler, ctx);
+        s.interrupt.set_handler_context(unsafe {
+            executor::raw::task_from_waker(&cx.waker()).cast().as_ptr()
+        });
         if s.interrupt.is_enabled() {
             Poll::Pending
         } else {