Example usage of `heapless::spsc` triggers UB?
cr1901 opened this issue · 2 comments
The split
example of heapless::spsc
attempts to avoid the need for using a Mutex<RefCell<Queue>>
_as well as avoiding static
scope for Producer
and Consumer
by using unsafe
code:
fn main() {
// NOTE(unsafe) beware of aliasing the `consumer` end point
let mut consumer = unsafe { Q.split().1 };
...
fn interrupt_handler() {
// NOTE(unsafe) beware of aliasing the `producer` end point
let mut producer = unsafe { Q.split().0 };
...
I'm not 100% sure, but I believe this usage is in and of itself UB; the lifetime of consumer
and producer
is tied to Q
, and the mutable borrow of Q
lasts as long as consumer
or producer
lives. If an interrupt happens while consumer
exists, Q
is mutably borrowed twice when producer
is created.
I would appreciate clarification, as I'm not 100% sure if this is UB (and if it is, what an alternate unsafe
solution should look like). Here is a playground snippet of me attempting to minimize the spsc
code to show UB in Miri: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=597cb05f9f1003515b01e82f79abb9c1
Indeed, this is probably UB.
One should split the queue while having it in static storage and then move the producer/consumer to their usage locations.
I also think it's UB, since this wouldn't be allowed even in a single thread if you paste it in the playground example
let mut q = Queue(0);
let producer = q.split().0;
let consumer = q.split().1;
producer.enqueue();
consumer.dequeue();
what an alternate
unsafe
solution should look like
I'm not saying it should look like this, this is the best I could come up with so far using an intermediary Mutex<Refcell<Option<_>>>
to pass the producer to the handler. Feels very boilerplatey though.
static PRODUCER: Mutex<RefCell<Option<Producer<'static, (), 4>>>> =
Mutex::new(RefCell::new(None));
fn main() {
let mut consumer = {
let (p, c) = {
static mut Q: Queue<(), 4> = Queue::new();
// SAFETY: Mutable access to `Q` is allowed exclusively in this scope
// and `main` is only called once.
unsafe { Q.split() }
};
critical_section::with(move |cs| {
let mut producer = PRODUCER.borrow_ref_mut(cs);
*producer = Some(p);
});
c
};
}
fn interrupt() {
let mut producer = {
static mut P: Option<Producer<'static, (), 4>> = None;
// SAFETY: Mutable access to `P` is allowed exclusively in this scope
// and `interrupt` cannot be called directly or preempt itself.
unsafe { &mut P }
}.get_or_insert_with(|| {
critical_section::with(|cs| {
PRODUCER.borrow_ref_mut(cs).take().unwrap()
})
});
}